Skip to content

Commit eeb76d3

Browse files
committed
[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
1 parent 9bdb580 commit eeb76d3

File tree

2 files changed

+44
-1
lines changed

2 files changed

+44
-1
lines changed

lib/websocket-server.js

+3-1
Original file line numberDiff line numberDiff line change
@@ -186,12 +186,14 @@ class WebSocketServer extends EventEmitter {
186186
req.headers['sec-websocket-key'] !== undefined
187187
? req.headers['sec-websocket-key'].trim()
188188
: false;
189+
const upgrade = req.headers.upgrade;
189190
const version = +req.headers['sec-websocket-version'];
190191
const extensions = {};
191192

192193
if (
193194
req.method !== 'GET' ||
194-
req.headers.upgrade.toLowerCase() !== 'websocket' ||
195+
upgrade === undefined ||
196+
upgrade.toLowerCase() !== 'websocket' ||
195197
!key ||
196198
!keyRegex.test(key) ||
197199
(version !== 8 && version !== 13) ||

test/websocket-server.test.js

+41
Original file line numberDiff line numberDiff line change
@@ -383,6 +383,47 @@ describe('WebSocketServer', () => {
383383
});
384384

385385
describe('Connection establishing', () => {
386+
it('fails if the Upgrade header field value cannot be read', (done) => {
387+
const server = http.createServer();
388+
const wss = new WebSocket.Server({ noServer: true });
389+
390+
server.maxHeadersCount = 1;
391+
392+
server.on('upgrade', (req, socket, head) => {
393+
assert.deepStrictEqual(req.headers, { foo: 'bar' });
394+
wss.handleUpgrade(req, socket, head, () => {
395+
done(new Error('Unexpected callback invocation'));
396+
});
397+
});
398+
399+
server.listen(() => {
400+
const req = http.get({
401+
port: server.address().port,
402+
headers: {
403+
foo: 'bar',
404+
bar: 'baz',
405+
Connection: 'Upgrade',
406+
Upgrade: 'websocket'
407+
}
408+
});
409+
410+
req.on('response', (res) => {
411+
assert.strictEqual(res.statusCode, 400);
412+
413+
const chunks = [];
414+
415+
res.on('data', (chunk) => {
416+
chunks.push(chunk);
417+
});
418+
419+
res.on('end', () => {
420+
assert.strictEqual(Buffer.concat(chunks).toString(), 'Bad Request');
421+
server.close(done);
422+
});
423+
});
424+
});
425+
});
426+
386427
it('fails if the Sec-WebSocket-Key header is invalid (1/2)', (done) => {
387428
const wss = new WebSocket.Server({ port: 0 }, () => {
388429
const req = http.get({

0 commit comments

Comments
 (0)