Skip to content
This repository has been archived by the owner on Sep 7, 2020. It is now read-only.

Vulnerable Regular Expression #4

Closed
cristianstaicu opened this issue Sep 8, 2017 · 16 comments
Closed

Vulnerable Regular Expression #4

cristianstaicu opened this issue Sep 8, 2017 · 16 comments

Comments

@cristianstaicu
Copy link

The following regular expression used in parsing the JSON file is vulnerable to ReDoS:

/\s+$/

The slowdown is moderately low: for 50.000 characters around 2 seconds matching time. However, I would still suggest one of the following:

  • remove the regex,
  • anchor the regex,
  • limit the number of characters that can be matched by the repetition,
  • limit the input size.

If needed, I can provide an actual example showing the slowdown.

@AliSawari
Copy link

@cristianstaicu Yeah I noticed that too , as Snyk warns me about them vulnerabilities 😄
I don't have a serious app using this lib but I'm concerned about further uses 🤔

@get will be there any patches ?!

@AliSawari
Copy link

parsejson is a Method that parses a JSON string and returns a JSON object.

Affected versions of this package are vulnerable to Regular expression Denial of Service (ReDoS) attacks. An attacker may pass a specially crafted JSON data, causing the server to hang.

The Regular expression Denial of Service (ReDoS) is a type of Denial of Service attack. Many Regular Expression implementations may reach extreme situations that cause them to work very slowly (exponentially related to input size), allowing an attacker to exploit this and can cause the program to enter these extreme situations by using a Regex string and cause the service to hang for a large periods of time.

text quoted from snyk vuln page : Link

@joshwiens
Copy link

It's also worth noting that anyone that uses snyk / nsp & socket.io-client has broken client builds

@AliSawari
Copy link

@d3viant0ne WOW , didn't expect that coming, Why's that?

@joshwiens
Copy link

If you run the nsp or snyk checks in a CI run as a part of your validations, you get a non zero exit code, thus broken as parsejson is a nested dependency of socket.io-client

@rgeraldporter
Copy link

Any plans to fix this?

@alexw10
Copy link

alexw10 commented Oct 27, 2017

Anyone have an update on this anyone? Need a fix soon since some packages we use are using this and is out of our control. Our company's security group is needing this fixed.

I am willing to help out as well and or try to fix this.

@CSobol
Copy link

CSobol commented Oct 30, 2017

Is there a reason you can't just replace line 20 with data = data.trim()?

I want to issue an MR, but if you have compatability concerns then it's not as simple I suppose.

@vaikzs
Copy link

vaikzs commented Jun 19, 2018

Is there a fix for this?

@soundasleep
Copy link

If you're coming in here from an audit log/warning: this package looks to be essentially unmaintained, and in any case your packages, and their dependencies, should be using JSON.parse(...) rather than this package.

You might need to hunt through your package-lock.json dependencies to find which dependency is relying on parsejson: in my case, I needed to upgrade browser-sync.

@HansUXdev
Copy link

@cristianstaicu you seem the most familiar with this. Would you (or anyone else) be willing to submit a PR with a patch and or fork the repo with a patch if this repo isn't maintained?

@kmturley
Copy link

kmturley commented Aug 7, 2018

In my case I did:
npm ls parsejson

Which outputted:

angular-ecommerce@0.0.0
└─┬ karma@1.7.1
  └─┬ socket.io@1.7.3
    └─┬ socket.io-client@1.7.3
      └─┬ engine.io-client@1.8.3
        └── parsejson@0.0.3

And fixed using:
npm install karma@2.0.0

athlonUA added a commit to lazy-ants/angular-universal that referenced this issue Aug 13, 2018
espadrine added a commit to espadrine/shields that referenced this issue Nov 3, 2018
This fixes remaining vulnerabilities raised by `npm audit`.

Follow-up to badges#2258.

Related issues from dependencies:

- camp upgrade: espadrine/sc#64
- socket.io vulnerability: galkn/parsejson#4
espadrine added a commit to espadrine/shields that referenced this issue Nov 4, 2018
This fixes remaining vulnerabilities raised by `npm audit`.

Follow-up to badges#2258.

Related issues from dependencies:

- camp upgrade: espadrine/sc#64
- socket.io vulnerability: galkn/parsejson#4
chris48s pushed a commit to badges/shields that referenced this issue Nov 4, 2018
This fixes remaining vulnerabilities raised by `npm audit`.

Follow-up to #2258.

Related issues from dependencies:

- camp upgrade: espadrine/sc#64
- socket.io vulnerability: galkn/parsejson#4
@Flink91
Copy link

Flink91 commented Jan 22, 2019

I had this problem - for future visitors:
If you are using the "socketio" package, change to "socket.io", those are different.

@tibbon
Copy link

tibbon commented Mar 15, 2019

This is a CVE with a HIGH rating, and is still open?

@pertrai1
Copy link

I am getting a HIGH rating on a project I started working on:

https://nvd.nist.gov/vuln/detail/CVE-2017-16113

@pcolusso
Copy link

Hey everyone, as mentioned above this package looks like it's clearly unmaintained. I recieved this secuirty warning as a knock-on affect of another unmaintained package. Was able to track down the root cause via yarn why recommend to anyone else viewing this to do the same, and cut out whatever package is requesting this. Yarn's Selective Dependency Resolutions is also quite helpful if you need to force override a package's version that may be still using this module.

@galkn galkn closed this as completed Sep 6, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests