[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

@@ -235,6 +235,7 @@ class WebSocketServer extends EventEmitter {
  socket.on('error', socketOnError);
 
  const key = req.headers['sec-websocket-key'];
+ const upgrade = req.headers.upgrade;
  const version = +req.headers['sec-websocket-version'];
 
  if (req.method !== 'GET') {
@@ -243,13 +244,13 @@ class WebSocketServer extends EventEmitter {
  return;
  }
 
- if (req.headers.upgrade.toLowerCase() !== 'websocket') {
+ if (upgrade === undefined || upgrade.toLowerCase() !== 'websocket') {
  const message = 'Invalid Upgrade header';
  abortHandshakeOrEmitwsClientError(this, req, socket, 400, message);
  return;
  }
 
- if (!key || !keyRegex.test(key)) {
+ if (key === undefined || !keyRegex.test(key)) {
  const message = 'Missing or invalid Sec-WebSocket-Key header';
  abortHandshakeOrEmitwsClientError(this, req, socket, 400, message);
  return;

lib/websocket.js

@@ -928,7 +928,9 @@ function initAsClient(websocket, address, protocols, options) {
 
  req = websocket._req = null;
 
- if (res.headers.upgrade.toLowerCase() !== 'websocket') {
+ const upgrade = res.headers.upgrade;
+
+ if (upgrade === undefined || upgrade.toLowerCase() !== 'websocket') {
  abortHandshake(websocket, socket, 'Invalid Upgrade header');
  return;
  }

test/websocket-server.test.js

@@ -653,6 +653,50 @@ describe('WebSocketServer', () => {
  });
  });
 
+ 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(),
+ 'Invalid Upgrade header'
+ );
+ server.close(done);
+ });
+ });
+ });
+ });
+
  it('fails if the Upgrade header field value is not "websocket"', (done) => {
  const wss = new WebSocket.Server({ port: 0 }, () => {
  const req = http.get({

test/websocket.test.js

@@ -757,6 +757,32 @@ describe('WebSocket', () => {
  beforeEach((done) => server.listen(0, done));
  afterEach((done) => server.close(done));
 
+ it('fails if the Upgrade header field value cannot be read', (done) => {
+ server.once('upgrade', (req, socket) => {
+ socket.on('end', socket.end);
+ socket.write(
+ 'HTTP/1.1 101 Switching Protocols\r\n' +
+ 'Connection: Upgrade\r\n' +
+ 'Upgrade: websocket\r\n' +
+ '\r\n'
+ );
+ });
+
+ const ws = new WebSocket(`ws:https://localhost:${server.address().port}`);
+
+ ws._req.maxHeadersCount = 1;
+
+ ws.on('upgrade', (res) => {
+ assert.deepStrictEqual(res.headers, { connection: 'Upgrade' });
+
+ ws.on('error', (err) => {
+ assert.ok(err instanceof Error);
+ assert.strictEqual(err.message, 'Invalid Upgrade header');
+ done();
+ });
+ });
+ });
+
  it('fails if the Upgrade header field value is not "websocket"', (done) => {
  server.once('upgrade', (req, socket) => {
  socket.on('end', socket.end);