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 36a3f4d commit 4abd8f6
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 @@ -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)
) {
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 @@ -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({
Expand Down

0 comments on commit 4abd8f6

Please sign in to comment.