Skip to content

Commit

Permalink
[major] Validate the Sec-WebSocket-Protocol header
Browse files Browse the repository at this point in the history
Abort the handshake if the `Sec-WebSocket-Protocol` header is invalid.
  • Loading branch information
lpinca committed Jul 14, 2021
1 parent 0aecf0c commit 1877dde
Show file tree
Hide file tree
Showing 7 changed files with 240 additions and 55 deletions.
2 changes: 1 addition & 1 deletion doc/ws.md
Original file line number Diff line number Diff line change
Expand Up @@ -120,7 +120,7 @@ if `verifyClient` is provided with two arguments then those are:

`handleProtocols` takes two arguments:

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

Expand Down
23 changes: 1 addition & 22 deletions lib/extension.js
Original file line number Diff line number Diff line change
@@ -1,27 +1,6 @@
'use strict';

//
// Allowed token characters:
//
// '!', '#', '$', '%', '&', ''', '*', '+', '-',
// '.', 0-9, A-Z, '^', '_', '`', a-z, '|', '~'
//
// tokenChars[32] === 0 // ' '
// tokenChars[33] === 1 // '!'
// tokenChars[34] === 0 // '"'
// ...
//
// prettier-ignore
const tokenChars = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0 - 15
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16 - 31
0, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 0, // 32 - 47
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, // 48 - 63
0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 64 - 79
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, // 80 - 95
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 96 - 111
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 1, 0 // 112 - 127
];
const { tokenChars } = require('./validation');

/**
* Adds an offer to the map of extension offers or a parameter to the map of
Expand Down
62 changes: 62 additions & 0 deletions lib/subprotocol.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,62 @@
'use strict';

const { tokenChars } = require('./validation');

/**
* Parses the `Sec-WebSocket-Protocol` header into a set of subprotocol names.
*
* @param {String} header The field value of the header
* @return {Set} The subprotocol names
* @public
*/
function parse(header) {
const protocols = new Set();
let start = -1;
let end = -1;
let i = 0;

for (i; i < header.length; i++) {
const code = header.charCodeAt(i);

if (end === -1 && tokenChars[code] === 1) {
if (start === -1) start = i;
} else if (
i !== 0 &&
(code === 0x20 /* ' ' */ || code === 0x09) /* '\t' */
) {
if (end === -1 && start !== -1) end = i;
} else if (code === 0x2c /* ',' */) {
if (start === -1) {
throw new SyntaxError(`Unexpected character at index ${i}`);
}

if (end === -1) end = i;

const protocol = header.slice(start, end);

if (protocols.has(protocol)) {
throw new SyntaxError(`The "${protocol}" subprotocol is duplicated`);
}

protocols.add(protocol);
start = end = -1;
} else {
throw new SyntaxError(`Unexpected character at index ${i}`);
}
}

if (start === -1 || end !== -1) {
throw new SyntaxError('Unexpected end of input');
}

const protocol = header.slice(start, i);

if (protocols.has(protocol)) {
throw new SyntaxError(`The "${protocol}" subprotocol is duplicated`);
}

protocols.add(protocol);
return protocols;
}

module.exports = { parse };
29 changes: 27 additions & 2 deletions lib/validation.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,28 @@
'use strict';

//
// Allowed token characters:
//
// '!', '#', '$', '%', '&', ''', '*', '+', '-',
// '.', 0-9, A-Z, '^', '_', '`', a-z, '|', '~'
//
// tokenChars[32] === 0 // ' '
// tokenChars[33] === 1 // '!'
// tokenChars[34] === 0 // '"'
// ...
//
// prettier-ignore
const tokenChars = [
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 0 - 15
0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, // 16 - 31
0, 1, 0, 1, 1, 1, 1, 1, 0, 0, 1, 1, 0, 1, 1, 0, // 32 - 47
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 0, 0, 0, // 48 - 63
0, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 64 - 79
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 0, 0, 1, 1, // 80 - 95
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, // 96 - 111
1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 1, 0, 1, 0, 1, 0 // 112 - 127
];

/**
* Checks if a status code is allowed in a close frame.
*
Expand Down Expand Up @@ -94,11 +117,13 @@ try {
isValidStatusCode,
isValidUTF8(buf) {
return buf.length < 150 ? _isValidUTF8(buf) : isValidUTF8(buf);
}
},
tokenChars
};
} catch (e) /* istanbul ignore next */ {
module.exports = {
isValidStatusCode,
isValidUTF8: _isValidUTF8
isValidUTF8: _isValidUTF8,
tokenChars
};
}
62 changes: 33 additions & 29 deletions lib/websocket-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,10 @@ const net = require('net');
const tls = require('tls');
const { createHash } = require('crypto');

const extension = require('./extension');
const PerMessageDeflate = require('./permessage-deflate');
const subprotocol = require('./subprotocol');
const WebSocket = require('./websocket');
const { format, parse } = require('./extension');
const { GUID, kWebSocket } = require('./constants');

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

const key =
req.headers['sec-websocket-key'] !== undefined
? req.headers['sec-websocket-key'].trim()
? req.headers['sec-websocket-key']
: false;
const version = +req.headers['sec-websocket-version'];
const extensions = {};
Expand All @@ -224,6 +225,17 @@ class WebSocketServer extends EventEmitter {
return abortHandshake(socket, 400);
}

const secWebSocketProtocol = req.headers['sec-websocket-protocol'];
let protocols = new Set();

if (secWebSocketProtocol !== undefined) {
try {
protocols = subprotocol.parse(secWebSocketProtocol);
} catch (err) {
return abortHandshake(socket, 400);
}
}

if (this.options.perMessageDeflate) {
const perMessageDeflate = new PerMessageDeflate(
this.options.perMessageDeflate,
Expand All @@ -232,7 +244,7 @@ class WebSocketServer extends EventEmitter {
);

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

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

this.completeUpgrade(key, extensions, req, socket, head, cb);
this.completeUpgrade(
extensions,
key,
protocols,
req,
socket,
head,
cb
);
});
return;
}

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

this.completeUpgrade(key, extensions, req, socket, head, cb);
this.completeUpgrade(extensions, key, protocols, req, socket, head, cb);
}

/**
* Upgrade the connection to WebSocket.
*
* @param {String} key The value of the `Sec-WebSocket-Key` header
* @param {Object} extensions The accepted extensions
* @param {String} key The value of the `Sec-WebSocket-Key` header
* @param {Set} protocols The subprotocols
* @param {http.IncomingMessage} req The request object
* @param {(net.Socket|tls.Socket)} socket The network socket between the
* server and client
Expand All @@ -284,7 +305,7 @@ class WebSocketServer extends EventEmitter {
* @throws {Error} If called more than once with the same socket
* @private
*/
completeUpgrade(key, extensions, req, socket, head, cb) {
completeUpgrade(extensions, key, protocols, req, socket, head, cb) {
//
// Destroy the socket if the client has already sent a FIN packet.
//
Expand All @@ -311,19 +332,14 @@ class WebSocketServer extends EventEmitter {
];

const ws = new WebSocket(null);
let protocol = req.headers['sec-websocket-protocol'];

if (protocol) {
protocol = protocol.split(',').map(trim);

if (protocols.size) {
//
// Optionally call external protocol selection handler.
//
if (this.options.handleProtocols) {
protocol = this.options.handleProtocols(protocol, req);
} else {
protocol = protocol[0];
}
const protocol = this.options.handleProtocols
? this.options.handleProtocols(protocols, req)
: protocols.values().next().value;

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

if (extensions[PerMessageDeflate.extensionName]) {
const params = extensions[PerMessageDeflate.extensionName].params;
const value = format({
const value = extension.format({
[PerMessageDeflate.extensionName]: [params]
});
headers.push(`Sec-WebSocket-Extensions: ${value}`);
Expand Down Expand Up @@ -433,15 +449,3 @@ function abortHandshake(socket, code, message, headers) {
socket.removeListener('error', socketOnError);
socket.destroy();
}

/**
* Remove whitespace characters from both ends of a string.
*
* @param {String} str The string
* @return {String} A new string representing `str` stripped of whitespace
* characters from both its beginning and end
* @private
*/
function trim(str) {
return str.trim();
}
91 changes: 91 additions & 0 deletions test/subprotocol.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
'use strict';

const assert = require('assert');

const { parse } = require('../lib/subprotocol');

describe('subprotocol', () => {
describe('parse', () => {
it('parses a single subprotocol', () => {
assert.deepStrictEqual(parse('foo'), new Set(['foo']));
});

it('parses multiple subprotocols', () => {
assert.deepStrictEqual(
parse('foo,bar,baz'),
new Set(['foo', 'bar', 'baz'])
);
});

it('ignores the optional white spaces', () => {
const header = 'foo , bar\t, \tbaz\t , qux\t\t,norf';

assert.deepStrictEqual(
parse(header),
new Set(['foo', 'bar', 'baz', 'qux', 'norf'])
);
});

it('throws an error if a subprotocol is empty', () => {
[
[',', 0],
['foo,,', 4],
['foo, ,', 6]
].forEach((element) => {
assert.throws(
() => parse(element[0]),
new RegExp(
`^SyntaxError: Unexpected character at index ${element[1]}$`
)
);
});
});

it('throws an error if a subprotocol is duplicated', () => {
['foo,foo,bar', 'foo,bar,foo'].forEach((header) => {
assert.throws(
() => parse(header),
/^SyntaxError: The "foo" subprotocol is duplicated$/
);
});
});

it('throws an error if a white space is misplaced', () => {
[
['f oo', 2],
[' foo', 0]
].forEach((element) => {
assert.throws(
() => parse(element[0]),
new RegExp(
`^SyntaxError: Unexpected character at index ${element[1]}$`
)
);
});
});

it('throws an error if a subprotocol contains invalid characters', () => {
[
['f@o', 1],
['f\\oo', 1],
['foo,b@r', 5]
].forEach((element) => {
assert.throws(
() => parse(element[0]),
new RegExp(
`^SyntaxError: Unexpected character at index ${element[1]}$`
)
);
});
});

it('throws an error if the header value ends prematurely', () => {
['foo ', 'foo, ', 'foo,bar ', 'foo,bar,'].forEach((header) => {
assert.throws(
() => parse(header),
/^SyntaxError: Unexpected end of input$/
);
});
});
});
});
Loading

0 comments on commit 1877dde

Please sign in to comment.