Throws TypeError when there are too many HTTP headers

[rrlapointe]

Is there an existing issue for this?

• I've searched for any related issues and avoided creating a duplicate issue.

Description

I use this package for a websocket server. In the process of changing some options to try to make my websocket server slightly more resistant to DoS, I lowered the maxHeadersCount field on the HTTP server from its default of 2000 to a lower value. I then tested to see whether the server would reject requests with too many headers. Instead, the server crashed due to a TypeError from websocket-server.js line 246. This seems like a DoS issue.

I found this similar issue: #1838

ws version

8.17.0

Node.js Version

v21.5.0

System

OS: Debian GNU/Linux 12 (bookworm)
CPU: x64 Intel Core i5 CPU
Memory: 2.62 GB / 6.58 GB
Container: Yes
Shell: /bin/bash

Expected result

I expected the websocket server to refuse the handshake. Maybe respond with a 400 Bad Request. I did not expect my server app to crash from a TypeError.

Actual result

webpack://somewhere/node_modules/ws/lib/websocket-server.js:246
    if (req.headers.upgrade.toLowerCase() !== 'websocket') {
                            ^


TypeError: Cannot read properties of undefined (reading 'toLowerCase')
    at handleUpgrade (webpack://somewhere/node_modules/ws/lib/websocket-server.js:246:29)
    at Server.upgrade (webpack://somewhere/node_modules/ws/lib/websocket-server.js:119:16)
    at Server.emit (node:events:519:28)
    at onParserExecuteCommon (node:_http_server:942:14)
    at onParserExecute (node:_http_server:836:3)

Node.js v21.5.0

Attachments

No response

[lpinca]

Please create a minimal reproducible test case using only ws. As per #1838 (comment), #1838 (comment) and #1838 (comment) it cannot happen unless ws is incorrectly used.

If the server 'upgrade' event is emitted without the Upgrade header then the bug is in Node.js core.

[lpinca]

Here is a test case to reproduce the issue using only Node.js core modules.

const http = require('http');

const server = http.createServer();

server.maxHeadersCount = 1;

server.on('upgrade', function (request) {
  console.log(request.headers);
});

server.listen(function () {
  const { port } = server.address();
  const request = http.request({
    headers: {
      foo: 'foo',
      bar: 'bar',
      baz: 'baz',
      qux: 'qux',
      connection: 'Upgrade',
      upgrade: 'protocol'
    },
    host: '127.0.0.1',
    port
  });

  request.end();
});

The 'upgrade' event is emitted even if server.maxHeadersCount is set to 1. It seems that setting the property does not abort the request. If it is the expected behavior, I wonder what is the purpose of setting that limit.

[lpinca]

See #2231.

[Maliksb11]

Ok