Skip to content

Commit 1877dde

Browse files
lpincalpinca
committedJul 14, 2021
[major] Validate the Sec-WebSocket-Protocol header
Abort the handshake if the `Sec-WebSocket-Protocol` header is invalid.
·
8.18.18.0.0
1 parent 0aecf0c commit 1877dde

7 files changed

+240
-55
lines changed
 

‎doc/ws.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -120,7 +120,7 @@ if `verifyClient` is provided with two arguments then those are:
120120

121121
`handleProtocols` takes two arguments:
122122

123-
- `protocols` {Array} The list of WebSocket subprotocols indicated by the client
123+
- `protocols` {Set} The list of WebSocket subprotocols indicated by the client
124124
in the `Sec-WebSocket-Protocol` header.
125125
- `request` {http.IncomingMessage} The client HTTP GET request.
126126

‎lib/extension.js

+1-22
Original file line numberDiff line numberDiff line change
@@ -1,27 +1,6 @@
11
'use strict';
22

3-
//
4-
// Allowed token characters:
5-
//
6-
// '!', '#', '$', '%', '&', ''', '*', '+', '-',
7-
// '.', 0-9, A-Z, '^', '_', '`', a-z, '|', '~'
8-
//
9-
// tokenChars[32] === 0 // ' '
10-
// tokenChars[33] === 1 // '!'
11-
// tokenChars[34] === 0 // '"'
12-
// ...
13-
//
14-
// prettier-ignore
15-
const tokenChars = [
16-
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0 - 15
17-
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16 - 31
18-
0, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 0, // 32 - 47
19-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, // 48 - 63
20-
0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 64 - 79
21-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, // 80 - 95
22-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 96 - 111
23-
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 1, 0 // 112 - 127
24-
];
3+
const { tokenChars } = require('./validation');
254

265
/**
276
* Adds an offer to the map of extension offers or a parameter to the map of

‎lib/subprotocol.js

+62
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,62 @@
1+
'use strict';
2+
3+
const { tokenChars } = require('./validation');
4+
5+
/**
6+
* Parses the `Sec-WebSocket-Protocol` header into a set of subprotocol names.
7+
*
8+
* @param {String} header The field value of the header
9+
* @return {Set} The subprotocol names
10+
* @public
11+
*/
12+
function parse(header) {
13+
const protocols = new Set();
14+
let start = -1;
15+
let end = -1;
16+
let i = 0;
17+
18+
for (i; i < header.length; i++) {
19+
const code = header.charCodeAt(i);
20+
21+
if (end === -1 && tokenChars[code] === 1) {
22+
if (start === -1) start = i;
23+
} else if (
24+
i !== 0 &&
25+
(code === 0x20 /* ' ' */ || code === 0x09) /* '\t' */
26+
) {
27+
if (end === -1 && start !== -1) end = i;
28+
} else if (code === 0x2c /* ',' */) {
29+
if (start === -1) {
30+
throw new SyntaxError(`Unexpected character at index ${i}`);
31+
}
32+
33+
if (end === -1) end = i;
34+
35+
const protocol = header.slice(start, end);
36+
37+
if (protocols.has(protocol)) {
38+
throw new SyntaxError(`The "${protocol}" subprotocol is duplicated`);
39+
}
40+
41+
protocols.add(protocol);
42+
start = end = -1;
43+
} else {
44+
throw new SyntaxError(`Unexpected character at index ${i}`);
45+
}
46+
}
47+
48+
if (start === -1 || end !== -1) {
49+
throw new SyntaxError('Unexpected end of input');
50+
}
51+
52+
const protocol = header.slice(start, i);
53+
54+
if (protocols.has(protocol)) {
55+
throw new SyntaxError(`The "${protocol}" subprotocol is duplicated`);
56+
}
57+
58+
protocols.add(protocol);
59+
return protocols;
60+
}
61+
62+
module.exports = { parse };

‎lib/validation.js

+27-2
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,28 @@
11
'use strict';
22

3+
//
4+
// Allowed token characters:
5+
//
6+
// '!', '#', '$', '%', '&', ''', '*', '+', '-',
7+
// '.', 0-9, A-Z, '^', '_', '`', a-z, '|', '~'
8+
//
9+
// tokenChars[32] === 0 // ' '
10+
// tokenChars[33] === 1 // '!'
11+
// tokenChars[34] === 0 // '"'
12+
// ...
13+
//
14+
// prettier-ignore
15+
const tokenChars = [
16+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0 - 15
17+
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16 - 31
18+
0, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 0, // 32 - 47
19+
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, // 48 - 63
20+
0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 64 - 79
21+
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, // 80 - 95
22+
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 96 - 111
23+
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 1, 0 // 112 - 127
24+
];
25+
326
/**
427
* Checks if a status code is allowed in a close frame.
528
*
@@ -94,11 +117,13 @@ try {
94117
isValidStatusCode,
95118
isValidUTF8(buf) {
96119
return buf.length < 150 ? _isValidUTF8(buf) : isValidUTF8(buf);
97-
}
120+
},
121+
tokenChars
98122
};
99123
} catch (e) /* istanbul ignore next */ {
100124
module.exports = {
101125
isValidStatusCode,
102-
isValidUTF8: _isValidUTF8
126+
isValidUTF8: _isValidUTF8,
127+
tokenChars
103128
};
104129
}

‎lib/websocket-server.js

+33-29
Original file line numberDiff line numberDiff line change
@@ -9,9 +9,10 @@ const net = require('net');
99
const tls = require('tls');
1010
const { createHash } = require('crypto');
1111

12+
const extension = require('./extension');
1213
const PerMessageDeflate = require('./permessage-deflate');
14+
const subprotocol = require('./subprotocol');
1315
const WebSocket = require('./websocket');
14-
const { format, parse } = require('./extension');
1516
const { GUID, kWebSocket } = require('./constants');
1617

1718
const keyRegex = /^[+/0-9A-Za-z]{22}==$/;
@@ -208,7 +209,7 @@ class WebSocketServer extends EventEmitter {
208209

209210
const key =
210211
req.headers['sec-websocket-key'] !== undefined
211-
? req.headers['sec-websocket-key'].trim()
212+
? req.headers['sec-websocket-key']
212213
: false;
213214
const version = +req.headers['sec-websocket-version'];
214215
const extensions = {};
@@ -224,6 +225,17 @@ class WebSocketServer extends EventEmitter {
224225
return abortHandshake(socket, 400);
225226
}
226227

228+
const secWebSocketProtocol = req.headers['sec-websocket-protocol'];
229+
let protocols = new Set();
230+
231+
if (secWebSocketProtocol !== undefined) {
232+
try {
233+
protocols = subprotocol.parse(secWebSocketProtocol);
234+
} catch (err) {
235+
return abortHandshake(socket, 400);
236+
}
237+
}
238+
227239
if (this.options.perMessageDeflate) {
228240
const perMessageDeflate = new PerMessageDeflate(
229241
this.options.perMessageDeflate,
@@ -232,7 +244,7 @@ class WebSocketServer extends EventEmitter {
232244
);
233245

234246
try {
235-
const offers = parse(req.headers['sec-websocket-extensions']);
247+
const offers = extension.parse(req.headers['sec-websocket-extensions']);
236248

237249
if (offers[PerMessageDeflate.extensionName]) {
238250
perMessageDeflate.accept(offers[PerMessageDeflate.extensionName]);
@@ -260,22 +272,31 @@ class WebSocketServer extends EventEmitter {
260272
return abortHandshake(socket, code || 401, message, headers);
261273
}
262274

263-
this.completeUpgrade(key, extensions, req, socket, head, cb);
275+
this.completeUpgrade(
276+
extensions,
277+
key,
278+
protocols,
279+
req,
280+
socket,
281+
head,
282+
cb
283+
);
264284
});
265285
return;
266286
}
267287

268288
if (!this.options.verifyClient(info)) return abortHandshake(socket, 401);
269289
}
270290

271-
this.completeUpgrade(key, extensions, req, socket, head, cb);
291+
this.completeUpgrade(extensions, key, protocols, req, socket, head, cb);
272292
}
273293

274294
/**
275295
* Upgrade the connection to WebSocket.
276296
*
277-
* @param {String} key The value of the `Sec-WebSocket-Key` header
278297
* @param {Object} extensions The accepted extensions
298+
* @param {String} key The value of the `Sec-WebSocket-Key` header
299+
* @param {Set} protocols The subprotocols
279300
* @param {http.IncomingMessage} req The request object
280301
* @param {(net.Socket|tls.Socket)} socket The network socket between the
281302
* server and client
@@ -284,7 +305,7 @@ class WebSocketServer extends EventEmitter {
284305
* @throws {Error} If called more than once with the same socket
285306
* @private
286307
*/
287-
completeUpgrade(key, extensions, req, socket, head, cb) {
308+
completeUpgrade(extensions, key, protocols, req, socket, head, cb) {
288309
//
289310
// Destroy the socket if the client has already sent a FIN packet.
290311
//
@@ -311,19 +332,14 @@ class WebSocketServer extends EventEmitter {
311332
];
312333

313334
const ws = new WebSocket(null);
314-
let protocol = req.headers['sec-websocket-protocol'];
315-
316-
if (protocol) {
317-
protocol = protocol.split(',').map(trim);
318335

336+
if (protocols.size) {
319337
//
320338
// Optionally call external protocol selection handler.
321339
//
322-
if (this.options.handleProtocols) {
323-
protocol = this.options.handleProtocols(protocol, req);
324-
} else {
325-
protocol = protocol[0];
326-
}
340+
const protocol = this.options.handleProtocols
341+
? this.options.handleProtocols(protocols, req)
342+
: protocols.values().next().value;
327343

328344
if (protocol) {
329345
headers.push(`Sec-WebSocket-Protocol: ${protocol}`);
@@ -333,7 +349,7 @@ class WebSocketServer extends EventEmitter {
333349

334350
if (extensions[PerMessageDeflate.extensionName]) {
335351
const params = extensions[PerMessageDeflate.extensionName].params;
336-
const value = format({
352+
const value = extension.format({
337353
[PerMessageDeflate.extensionName]: [params]
338354
});
339355
headers.push(`Sec-WebSocket-Extensions: ${value}`);
@@ -433,15 +449,3 @@ function abortHandshake(socket, code, message, headers) {
433449
socket.removeListener('error', socketOnError);
434450
socket.destroy();
435451
}
436-
437-
/**
438-
* Remove whitespace characters from both ends of a string.
439-
*
440-
* @param {String} str The string
441-
* @return {String} A new string representing `str` stripped of whitespace
442-
* characters from both its beginning and end
443-
* @private
444-
*/
445-
function trim(str) {
446-
return str.trim();
447-
}

‎test/subprotocol.test.js

+91
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,91 @@
1+
'use strict';
2+
3+
const assert = require('assert');
4+
5+
const { parse } = require('../lib/subprotocol');
6+
7+
describe('subprotocol', () => {
8+
describe('parse', () => {
9+
it('parses a single subprotocol', () => {
10+
assert.deepStrictEqual(parse('foo'), new Set(['foo']));
11+
});
12+
13+
it('parses multiple subprotocols', () => {
14+
assert.deepStrictEqual(
15+
parse('foo,bar,baz'),
16+
new Set(['foo', 'bar', 'baz'])
17+
);
18+
});
19+
20+
it('ignores the optional white spaces', () => {
21+
const header = 'foo , bar\t, \tbaz\t , qux\t\t,norf';
22+
23+
assert.deepStrictEqual(
24+
parse(header),
25+
new Set(['foo', 'bar', 'baz', 'qux', 'norf'])
26+
);
27+
});
28+
29+
it('throws an error if a subprotocol is empty', () => {
30+
[
31+
[',', 0],
32+
['foo,,', 4],
33+
['foo, ,', 6]
34+
].forEach((element) => {
35+
assert.throws(
36+
() => parse(element[0]),
37+
new RegExp(
38+
`^SyntaxError: Unexpected character at index ${element[1]}$`
39+
)
40+
);
41+
});
42+
});
43+
44+
it('throws an error if a subprotocol is duplicated', () => {
45+
['foo,foo,bar', 'foo,bar,foo'].forEach((header) => {
46+
assert.throws(
47+
() => parse(header),
48+
/^SyntaxError: The "foo" subprotocol is duplicated$/
49+
);
50+
});
51+
});
52+
53+
it('throws an error if a white space is misplaced', () => {
54+
[
55+
['f oo', 2],
56+
[' foo', 0]
57+
].forEach((element) => {
58+
assert.throws(
59+
() => parse(element[0]),
60+
new RegExp(
61+
`^SyntaxError: Unexpected character at index ${element[1]}$`
62+
)
63+
);
64+
});
65+
});
66+
67+
it('throws an error if a subprotocol contains invalid characters', () => {
68+
[
69+
['f@o', 1],
70+
['f\\oo', 1],
71+
['foo,b@r', 5]
72+
].forEach((element) => {
73+
assert.throws(
74+
() => parse(element[0]),
75+
new RegExp(
76+
`^SyntaxError: Unexpected character at index ${element[1]}$`
77+
)
78+
);
79+
});
80+
});
81+
82+
it('throws an error if the header value ends prematurely', () => {
83+
['foo ', 'foo, ', 'foo,bar ', 'foo,bar,'].forEach((header) => {
84+
assert.throws(
85+
() => parse(header),
86+
/^SyntaxError: Unexpected end of input$/
87+
);
88+
});
89+
});
90+
});
91+
});

‎test/websocket-server.test.js

+25-1
Original file line numberDiff line numberDiff line change
@@ -582,6 +582,30 @@ describe('WebSocketServer', () => {
582582
});
583583
});
584584

585+
it('fails is the Sec-WebSocket-Protocol header is invalid', (done) => {
586+
const wss = new WebSocket.Server({ port: 0 }, () => {
587+
const req = http.get({
588+
port: wss.address().port,
589+
headers: {
590+
Connection: 'Upgrade',
591+
Upgrade: 'websocket',
592+
'Sec-WebSocket-Key': 'dGhlIHNhbXBsZSBub25jZQ==',
593+
'Sec-WebSocket-Version': 13,
594+
'Sec-WebSocket-Protocol': 'foo;bar'
595+
}
596+
});
597+
598+
req.on('response', (res) => {
599+
assert.strictEqual(res.statusCode, 400);
600+
wss.close(done);
601+
});
602+
});
603+
604+
wss.on('connection', () => {
605+
done(new Error("Unexpected 'connection' event"));
606+
});
607+
});
608+
585609
it('fails if the Sec-WebSocket-Extensions header is invalid', (done) => {
586610
const wss = new WebSocket.Server(
587611
{
@@ -920,7 +944,7 @@ describe('WebSocketServer', () => {
920944
const handleProtocols = (protocols, request) => {
921945
assert.ok(request instanceof http.IncomingMessage);
922946
assert.strictEqual(request.url, '/');
923-
return protocols.pop();
947+
return Array.from(protocols).pop();
924948
};
925949
const wss = new WebSocket.Server({ handleProtocols, port: 0 }, () => {
926950
const ws = new WebSocket(`ws://localhost:${wss.address().port}`, [

0 commit comments

Comments
 (0)
Please sign in to comment.