From 4abd8f6de4b0b65ef80b3ff081989479ed93377e Mon Sep 17 00:00:00 2001 From: Luigi Pinca Date: Sun, 16 Jun 2024 12:25:32 +0200 Subject: [PATCH] [security] Fix crash when the Upgrade header cannot be read (#2231) 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 --- lib/websocket-server.js | 4 +++- test/websocket-server.test.js | 41 +++++++++++++++++++++++++++++++++++ 2 files changed, 44 insertions(+), 1 deletion(-) diff --git a/lib/websocket-server.js b/lib/websocket-server.js index 36820e114..a288a32d9 100644 --- a/lib/websocket-server.js +++ b/lib/websocket-server.js @@ -161,11 +161,13 @@ class WebSocketServer extends EventEmitter { handleUpgrade (req, socket, head, cb) { socket.on('error', socketOnError); + const upgrade = req.headers.upgrade; const version = +req.headers['sec-websocket-version']; const extensions = {}; if ( - req.method !== 'GET' || req.headers.upgrade.toLowerCase() !== 'websocket' || + req.method !== 'GET' || upgrade === undefined || + upgrade.toLowerCase() !== 'websocket' || !req.headers['sec-websocket-key'] || (version !== 8 && version !== 13) || !this.shouldHandle(req) ) { diff --git a/test/websocket-server.test.js b/test/websocket-server.test.js index 45d287978..832354cb7 100644 --- a/test/websocket-server.test.js +++ b/test/websocket-server.test.js @@ -364,6 +364,47 @@ describe('WebSocketServer', function () { }); describe('Connection establishing', function () { + it('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', function (done) { const wss = new WebSocket.Server({ port: 0 }, () => { const req = http.get({