Bump braces package to 3.0.3. Regenerate package-lock.json

[Furze]

Stops referencing a version of braces which contains a vulnerability https://security.snyk.io/package/npm/braces/3.0.2

[Furze]

Note: I updated package-lock.json, because it is still included although always generating it is off.

[paulmillr]

Why is this a problem?

[Furze]

At a high level, I care about this as I am expected to not have any packages in our dependencies tree that have "High" or higher vulnerabilities in my bill of materials.

Looking at how braces is being used in chokidar

https://github.com/paulmillr/chokidar/blob/7c50e25d10a497ce4409f6e52eb630f0d7647b97/index.js#L258 https://github.com/paulmillr/chokidar/blob/7c50e25d10a497ce4409f6e52eb630f0d7647b97/index.js#L215 https://github.com/paulmillr/chokidar/blob/7c50e25d10a497ce4409f6e52eb630f0d7647b97/index.js#L207

Someone could pass in a broken path which could trigger this vulnerability in braces.

[y-nk]

that'd be nice yeah.

[paulmillr]

Asking again. Why is this a problem?

https://stackoverflow.com/questions/22343224/whats-the-difference-between-tilde-and-caret-in-package-json

[atxslater]

Hi @paulmillr, Seconding what Furze was saying about 3.0.2: The braces package is vulnerable to Denial of Service (DoS) attacks. The parse function in the parse.js file does not sufficiently balance opening and closing curly braces ({, }) in user provided input. A remote attacker can leverage this vulnerability by supplying specially crafted input containing imbalanced braces to the affected functionality. This will result in the parser to enter a loop, resulting in memory exhaustion and ultimately a DoS condition.

[TroyCornwall]

I get what @paulmillr is getting at.

Chokidar referencing ~3.0.2 allows both 3.0.2 and 3.0.3, and when only having a dependency of chokidar npm ls braces does resolve to 3.0.3

I am seeing another package in my bill of materials which is using ^3.0.2, which should also allow both 3.0.2 and 3.0.3 (and 3.1.x in the future)

Which combined npm seems to be resolving these to 3.0.2, and a work around is to use npm force resolutions to force it to 3.0.3.

I am still of the opinion of updating the reference to ~3.0.3 to specifically avoid consumers of chokidar pulling in a vulnerable library.

[paulmillr]

I am seeing another package in my bill of materials which is using ^3.0.2, which should also allow both 3.0.2 and 3.0.3 (and 3.1.x in the future)

Which combined npm seems to be resolving these to 3.0.2

Why? Does this always happen or is limited to your setup? Are you able to debug this behavior?

[y-nk]

Which combined npm seems to be resolving these to 3.0.2, and a work around is to use npm force resolutions to force it to 3.0.3.

deps are deduped to the lowest compatible, it seems - but it'd be introduced by other-than-chokidar packages ; meaning that technically even if this PR would pass, issue would remain.

but i agree with @TroyCornwall here:

I am still of the opinion of updating the reference to ~3.0.3 to specifically avoid consumers of chokidar pulling in a vulnerable library.

PR is ready, it doesn't cost much to push the button and be done with it.

[paulmillr]

Long story short, "security researchers" made us (chokidar and braces maintainers) spend a shitton of time on an issue which they thought was critical, while in reality it's low-severity. In fact, they have since decreased the severity of the issue from critical to medium.

There has been no real-world exploit presented. There is PoC which "COULD HAPPEN" but it's highly unlikely bc a user would need to specifically craft bad pattern.

I'm sad about the whole situation. Some random company creates fake vulnerability and then we need to spend unreasonable amount of unpaid time resolving it. Users think that it's important because scanners harrass their build processes. No one understands version ranges etc.

even if this PR would pass, issue would remain

So, why would we merge the PR then?

PR is ready, it doesn't cost much to push the button and be done with it.

Then another "vulnerability" (which is not) would appear and we would need to do another release.

The issue needs to be resolved on a deeper level. Specifically, i've always thought that deps are resolved to the latest version - not to the earliest one. NPM issue needs to be created to investigate the issue further.

To reiterate: real vulnerabilities must be resolved and pushed ASAP. Shit vulnerabilities don't need to be resolved or pushed. The companies responsible for this shit need to be held accountable. Vulnerability scanners spitting fake CVEs back and forth need to be held accountable.

[TroyCornwall]

Here is a PoC showing the same vulnerability exists in chokidar if it uses the 3.0.2 version of braces.

it('braces issue', async () => {
    const maxRepeats = 10;
    for (let repeats = 1; repeats <= maxRepeats; repeats += 1) {
      const payload = `${'{/'.repeat(repeats*999)}/*/`;
      console.log(`Testing with ${repeats} repeats...`);
      const startTime = Date.now();
      chokidar_watch(payload, options);
      const endTime = Date.now();
      const executionTime = endTime - startTime;
      console.log(`Regex executed in ${executionTime / 1000}s.\n`);
      executionTime.should.be.lessThan(100);
    }
  });

As you can see in this image the time to parse the path does not scale linearly like you would expect. 100ms to pass a path is a very long time. A malicious user can use this to create a DoS where a user can input a malicious path.

If i extend that execution timeout and the number of repeats, you can see how bad the problem can be is -

[paulmillr]

@TroyCornwall how the fuck is this "poc" a real-world scenario? Here's another POC DOS for you:

while (true) {
  console.log(1)
}

A malicious user can use this to create a DoS where a user can input a malicious path.

Where? Point to exact steps a "malicious user" needs to do. Because as a user of braces I sure won't create 90000-character pattern and pass it by myself to braces.

https://github.com/paulmillr/chokidar/blob/7c50e25d10a497ce4409f6e52eb630f0d7647b97/index.js#L258