🔥NPM audit reports Denial of Service vulnerability

[Dangoo]

Hi, just wanted to let you know, that of today NPM released an advisory that serialize-to-js is vulnerable to Denial of Service attacks with a high severity.

https://npmjs.com/advisories/790

Would be great if you could look into it and hopefully resolve the vulnerability soon :)

[commenthol]

Hi @Dangoo, thanks for letting me know. Unfortunately no one got in touch with me reporting any issue as stated at npm.
Up to the moment I have no clue what the finding is about, how to determine and verify the issue stated, and how to perform that dos attack.
With the nothing I have currently it will be difficult to track things down. Usually I get notified on such topics with some insights.

[commenthol]

Mail was sent to support@npmjs.com - hopefully I will get a report on the allegation.

[commenthol]

Human errors may happen in human environments.
Albeit the advisory has been withdrawn, the issue is still present, so here the full disclosure.

---

Deserialization of untrusted data using this package is dangerous. Even though it uses safer-eval to try and sandbox evaluated code, it is vulnerable to DoS by deserializing 'function(){while(true){}}()'.

Minimal viable sample:

const serializeToJs = require('serialize-to-js')
var str = 'function(){while(true){}}()'
var res = serializeToJs.deserialize(str) // will never finish
console.log(res)

---

As I will not be able to fix the issue I'd propose the following option:

1. Breaking change: Remove the deserialize method from this package

2. Put a statement in safer-eval and recommend running the deserialization in a seperate worker thread with time based termination.

Any other ideas?

[Dor-Tumarkin]

I disclosed this issue via npmjs.com, assuming they will contact the package owner. I guess they didn't... Sorry it turned out this way.

A worker thread with time based termination is probably the best option, though this has to be min-maxed in some way - after all, if the timer is too long an attacker can still throttle their target by creating a ton of worker threads, and if it is too short legitimate work might terminate too early. Maybe create a limit on how many worker threads can be created, and possibly even a queue, to protect against the former. It would still restrict the service, but at least it won't crash or thrash when flooded.

[commenthol]

v2.0.0 released

https://github.com/commenthol/serialize-to-js/releases/tag/v2.0.0

[franciscolourenco]

Could another option be to serialize functions into a string representation, and not deserialize them? This would be more useful than no deserialization.

[salientknight]

At the moment upgrading to >=2.0.0 removes the vulnerability but then installing eslint brings it right back.

Here is my dependency hell:
npm i serialize-to-js>=2.0.0
npm WARN @typescript-eslint/eslint-plugin@1.13.0 requires a peer of eslint@^5.0.0 but none is installed. You must install peer dependencies yourself.
npm WARN @typescript-eslint/parser@1.13.0 requires a peer of eslint@^5.0.0 but none is installed. You must install peer dependencies yourself.

Updating eslint adds the audit warning back for serialize-to-js

[commenthol]

Hi @salientknight,
version >=2.0.0 does not contain any longer code which is subject to the advised DoS, therefore the breaking change.
Running npm audit should usually give you the reason for the module being subject to the given advisory.
I recommend in your case:

1. delete node_modules folder
2. make a clean npm install
3. if you don't have a package-lock.json file run npm install --package-lock-only
4. then run npm audit

[salientknight]

Running npm audit should usually give you the reason for the module being subject to the given advisory.

yeah I was able to update to version 3 but Parcel seems to using version <2 and claims it's a NPM false report issue. Either way, updating causes a warning that eslint@^5.0.0 is not installed (as noted above).

I have done a new npm i and deleted json and I just seem locked a circle.

[commenthol]

Closing this issue as modules have been separated.