awslabs / aws-crt-nodejs

NodeJS bindings for the AWS Common Runtime.
Apache License 2.0
40 stars 25 forks source link

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

Closed abarke closed 17 hours ago

abarke commented 1 month ago

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               │ . > aws-iot-device-sdk-v2@1.20.0 > aws-crt@1.21.2 >    │
│                     │ @httptoolkit/websocket-stream@6.0.1 >                  │
│                     │ isomorphic-ws@4.0.1 > ws@8.17.0                        │
│                     │                                                        │
│                     │ . > aws-iot-device-sdk-v2@1.20.0 > aws-crt@1.21.2 >    │
│                     │ @httptoolkit/websocket-stream@6.0.1 > ws@8.17.0        │
│                     │                                                        │
│                     │ . > nuxt@3.11.2 > @nuxt/devtools@1.3.2 > ws@8.17.0     │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ 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               │ . > aws-iot-device-sdk-v2@1.20.0 > aws-crt@1.21.2 >    │
│                     │ mqtt@4.3.8 > ws@7.5.9                                  │
├─────────────────────┼────────────────────────────────────────────────────────┤
│ 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 ws@7.5.9 and ws@8.17.0

aws-iot-device-sdk-v2@1.20.0 > aws-crt@1.21.2 > @httptoolkit/websocket-stream@6.0.1 > isomorphic-ws@4.0.1 > ws@8.17.0
aws-iot-device-sdk-v2@1.20.0 > aws-crt@1.21.2 > mqtt@4.3.8 > ws@7.5.9

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 ws@8.17.1 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 commented 1 month ago

downstream issue https://github.com/aws/aws-iot-device-sdk-js-v2/issues/517

jmklix commented 1 month ago

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.

abarke commented 1 month ago

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 mqtt@4.3.8 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 commented 1 month ago

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 mqtt@4.3.8 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 commented 17 hours ago

https://github.com/awslabs/aws-crt-nodejs/pull/562