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

Remove parsejson NPM package #580

Merged
merged 1 commit into from
Sep 27, 2017
Merged

Conversation

mattgrande
Copy link

Prevents a potential ReDoS. See here: https://nodesecurity.io/advisories/528

I couldn't find any benefits to using parsejson; It seems to just call JSON.parse if it exists, and it exists in every node version, and every browser released since 2008.

Should I run the make and include it in this PR, or is that done by a build server or something along those lines?

The kind of change this PR does introduce

  • a bug fix
  • a new feature
  • an update to the documentation
  • a code change that improves performance
  • other

Current behaviour

Potential ReDoS for large strings due to parsejson package.

New behaviour

Removes parsejson in favour of native parsing.

Other information (e.g. related issues)

Reported here: #579

Prevents a potential ReDoS. See here: https://nodesecurity.io/advisories/528

I couldn't find any benefits to using parsejson; It seems to just call JSON.parse if it exists, and it exists in every node version, and every browser released since 2008.
@mattgrande
Copy link
Author

Thanks @kaarelkk. Should I run the make and include it in this PR, or is that done by a build server or something along those lines?

@justingreenberg
Copy link

@nuclearace @darrachequesne any shot at getting this merged and release a patch ASAP? the advisory #528 was released last Thu at 12:16 AM EST... as both NSP and Snyk are failing, i assume this is causing logjam in many CI/CD pipelines

vulnerable dependency path:

socket.io@2.0.3 > socket.io-client@2.0.3 > engine.io-client@3.1.1 > parsejson@0.0.3

the socket.io-client dependency on engine.io-client version is declared with tilde (~3.1.1) so a patch to engine.io-client should propagate up chain and resolve issue

@darrachequesne
Copy link
Member

Thanks!

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

Successfully merging this pull request may close these issues.

4 participants