Skip to content

Commit

Permalink
[security] Fix crash when the Upgrade header cannot be read (#2231)
Browse files Browse the repository at this point in the history
It is possible that the Upgrade header is correctly received and handled
(the `'upgrade'` event is emitted) without its value being returned to
the user. This can happen if the number of received headers exceed the
`server.maxHeadersCount` or `request.maxHeadersCount` threshold. In this
case `incomingMessage.headers.upgrade` may not be set.

Handle the case correctly and abort the handshake.

Fixes #2230
  • Loading branch information
lpinca committed Jun 16, 2024
1 parent 9bdb580 commit 85783fb
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 1 deletion.
4 changes: 3 additions & 1 deletion lib/websocket-server.js
Original file line number Diff line number Diff line change
Expand Up @@ -186,12 +186,14 @@ class WebSocketServer extends EventEmitter {
req.headers['sec-websocket-key'] !== undefined
? req.headers['sec-websocket-key'].trim()
: false;
const upgrade = req.headers.upgrade;
const version = +req.headers['sec-websocket-version'];
const extensions = {};

if (
req.method !== 'GET' ||
req.headers.upgrade.toLowerCase() !== 'websocket' ||
upgrade === undefined ||
upgrade.toLowerCase() !== 'websocket' ||
!key ||
!keyRegex.test(key) ||
(version !== 8 && version !== 13) ||
Expand Down
41 changes: 41 additions & 0 deletions test/websocket-server.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -383,6 +383,47 @@ describe('WebSocketServer', () => {
});

describe('Connection establishing', () => {
it.only('fails if the Upgrade header field value cannot be read', (done) => {
const server = http.createServer();
const wss = new WebSocket.Server({ noServer: true });

server.maxHeadersCount = 1;

server.on('upgrade', (req, socket, head) => {
assert.deepStrictEqual(req.headers, { foo: 'bar' });
wss.handleUpgrade(req, socket, head, () => {
done(new Error('Unexpected callback invocation'));
});
});

server.listen(() => {
const req = http.get({
port: server.address().port,
headers: {
foo: 'bar',
bar: 'baz',
Connection: 'Upgrade',
Upgrade: 'websocket'
}
});

req.on('response', (res) => {
assert.strictEqual(res.statusCode, 400);

const chunks = [];

res.on('data', (chunk) => {
chunks.push(chunk);
});

res.on('end', () => {
assert.strictEqual(Buffer.concat(chunks).toString(), 'Bad Request');
server.close(done);
});
});
});
});

it('fails if the Sec-WebSocket-Key header is invalid (1/2)', (done) => {
const wss = new WebSocket.Server({ port: 0 }, () => {
const req = http.get({
Expand Down

0 comments on commit 85783fb

Please sign in to comment.