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

npm audit - 2 vulnerabilities found - Severity: 2 high #556

Closed
abarke opened this issue Jul 16, 2024 · 5 comments
Closed

npm audit - 2 vulnerabilities found - Severity: 2 high #556

abarke opened this issue Jul 16, 2024 · 5 comments
Labels
bug This issue is a bug. dependencies This issue is a problem in a dependency. p3 This is a minor priority issue

Comments

@abarke
Copy link

abarke commented Jul 16, 2024

Describe the bug

$ pnpm audit
┌─────────────────────┬────────────────────────────────────────────────────────┐
│ high                │ ws affected by a DoS when handling a request with many │
│                     │ HTTP headers                                           │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Package             │ ws                                                     │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Vulnerable versions │ >=8.0.0 <8.17.1                                        │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Patched versions    │ >=8.17.1                                               │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Paths               │ . > [email protected] > [email protected] >    │
│                     │ @httptoolkit/[email protected] >                  │
│                     │ [email protected] > [email protected]                        │
│                     │                                                        │
│                     │ . > [email protected] > [email protected] >    │
│                     │ @httptoolkit/[email protected] > [email protected]        │
│                     │                                                        │
│                     │ . > [email protected] > @nuxt/[email protected] > [email protected]     │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ More info           │ https://github.com/advisories/GHSA-3h5v-q93c-6h6q      │
└─────────────────────┴────────────────────────────────────────────────────────┘
┌─────────────────────┬────────────────────────────────────────────────────────┐
│ high                │ ws affected by a DoS when handling a request with many │
│                     │ HTTP headers                                           │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Package             │ ws                                                     │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Vulnerable versions │ >=7.0.0 <7.5.10                                        │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Patched versions    │ >=7.5.10                                               │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ Paths               │ . > [email protected] > [email protected] >    │
│                     │ [email protected] > [email protected]                                  │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ More info           │ https://github.com/advisories/GHSA-3h5v-q93c-6h6q      │
└─────────────────────┴────────────────────────────────────────────────────────┘
2 vulnerabilities found
Severity: 2 high

Expected Behavior

No vulnerabilities found

Current Behavior

2 vulnerabilities found
Severity: 2 high

Reproduction Steps

npm audit

Possible Solution

Seems aws-crt is using 2 different major versions of [email protected] and [email protected]

Solution would be to bump to mqtt@>=5.7.2
Ref to "ws": "^8.17.1": https://github.com/mqttjs/MQTT.js/blob/v5.7.2/package.json#L127

Looking at the breaking changes from mqtt 4 > 5 these are only small changes needed/documented: https://github.com/mqttjs/MQTT.js/blob/v5.7.2/CHANGELOG.md

BREAKING CHANGES
- when creating an MqttClient instance 'new' is now required
- Dropped support for NodeJS 12-14

This should solve both issues by using a single [email protected] as isomorphic-ws depends on any version being a peerDependency.
https://github.com/heineiuo/isomorphic-ws/blob/master/package.json#L27

Additional Information/Context

No response

aws-crt-nodejs version used

1.21.2

nodejs version used

22.1.0

Operating System and version

Windows 11

@abarke abarke added bug This issue is a bug. needs-triage This issue or PR still needs to be triaged. labels Jul 16, 2024
@abarke
Copy link
Author

abarke commented Jul 16, 2024

downstream issue aws/aws-iot-device-sdk-js-v2#517

@jmklix
Copy link
Member

jmklix commented Jul 22, 2024

Thanks for pointing this out to us. It is currently not an issue for anyone using this sdk, as security vulnerabilities don't affect any of the functions used by this sdk. We will leave this issue open for when we update to the latest ws version.

@jmklix jmklix added dependencies This issue is a problem in a dependency. p3 This is a minor priority issue and removed needs-triage This issue or PR still needs to be triaged. labels Jul 22, 2024
@abarke
Copy link
Author

abarke commented Jul 23, 2024

Thanks for pointing this out to us. It is currently not an issue for anyone using this sdk, as security vulnerabilities don't affect any of the functions used by this sdk. We will leave this issue open for when we update to the latest ws version.

This broke our CI pipeline as we run npm audit as a task. So for now we just ignore it, but for some orgs this might become an issue and break the pipeline. Could you expand on how the security vulnerabilities doesn't affect any of the functions used by this sdk? 🤔

I would assume the [email protected] dependency is used by aws-iot-device-sdk-js-v2 as this is a core feature of using AWS IoT. We use the ws client for connecting MQTT via WebSocket to AWS IoT Core.

@bretambrose
Copy link
Contributor

Thanks for pointing this out to us. It is currently not an issue for anyone using this sdk, as security vulnerabilities don't affect any of the functions used by this sdk. We will leave this issue open for when we update to the latest ws version.

This broke our CI pipeline as we run npm audit as a task. So for now we just ignore it, but for some orgs this might become an issue and break the pipeline. Could you expand on how the security vulnerabilities doesn't affect any of the functions used by this sdk? 🤔

I would assume the [email protected] dependency is used by aws-iot-device-sdk-js-v2 as this is a core feature of using AWS IoT. We use the ws client for connecting MQTT via WebSocket to AWS IoT Core.

From the Github advisory:

A request with a number of headers exceeding the[server.maxHeadersCount](https://nodejs.org/api/http.html#servermaxheaderscount) threshold could be used to crash a ws server.

The vulnerability is in server functionality: handling the headers of an unknown http request (the websocket handshake). We do not use server functionality and the headers in the handshake are either under client control (the request) or IoT Core control (the response).

@bretambrose
Copy link
Contributor

#562

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug This issue is a bug. dependencies This issue is a problem in a dependency. p3 This is a minor priority issue
Projects
None yet
Development

No branches or pull requests

3 participants