fix: vulnerability

[coderaiser]

Fixes #36
https://security.snyk.io/vuln/SNYK-JS-BRACES-6838727

• ✅ Split to a couple chunks, which is better for speed in modern engines;
• ✅ Move out nested loop;
• ✅ Flatten blocks, then mark imbalanced;
• ✅ Remove parents from blocks, since they don't needed in flatten structure (less memory consumption);

[cichelero]

This questionable security issue relates to exponential complexity when parsing { or } characters to sanitize the input.

Given the current code still runs pretty easily with 10k brackets and this is way more than any practical use case, we could simply an additional validation for the max number of brackets around here:

braces/lib/parse.js

Lines 38 to 40 in 98414f9

	if (input.length > max) {
	throw new SyntaxError(`Input length (${input.length}), exceeds max characters (${max})`);
	}

[coderaiser]

Any progress on this? Can it be merged?

[NitishGameChanges]

When this PR will get Merged?

[alexcmetro]

Waiting for this PR to be merged.

[paulmillr]

PoC

for (let repeats = 1; repeats <= maxRepeats; repeats += 1) {
  const payload = '{'.repeat(repeats*90000);

  console.log(`Testing with ${repeats} repeats...`);
  const startTime = Date.now();
  braces(payload);
  const endTime = Date.now();
  const executionTime = endTime - startTime;
  console.log(`Regex executed in ${executionTime / 1000}s.\n`);
} 

[jonschlinkert]

First, I want to mention that I don't take this lightly and I do think it's very important to resolve vulnerabilities quickly. I've just reviewed the PR, which probably shouldn't have been merged, and I very carefully reviewed the code and the snyk report.

The problem

• There are 2,162,688 valid characters in JavaScript. See this gist
• This PR provides a way of preventing a specific character from being used more than maxSymbols times in the string. Which means you'd be able to pass 1024 characters 2,162,688 times (that is, if the maxLength option didn't already exist).

Since the PR won't solve the problem, and it introduces some performance degradations (like looping over the entire string again instead of counting in the parser where it's already done; not breaking when the max is reached, etc), I'm not going to publish this.

I would, however, consider adding support for process.env.MAX_BRACES_LENGTH or something like that, so that you can guarantee that this is respected by any braces in the tree (but that will only work for the new version and higher). Another thing I would consider is just setting the maxLength to a much lower default number, like 10,000, which braces handles in 0.008s.

The bigger problem

1. AFAIK (please correct me if I'm wrong), this particular "vulnerability" could only be exploited by someone who controls your dependencies, or a team member? I'm interested in understanding how a supply chain library that is almost exclusively used in devDependencies would be exploited by someone who isn't in control of the code.
2. If an implementor somewhere in the dependency tree was so inclined, there are much easier ways to cause catastrophic problems that defining inefficient brace patterns, which I'll not list here, and there is no way to prevent that from happening (see this gist).

Moving forward

I'd be happy to merge in a PR that does something like what I suggested earlier.

As final thoughts, these types of reports place a huge burden on maintainers like me, and while both snyk and the vulnerability-finders get paid a lot of $ for finding any possible thing that can be labeled a vulnerability (and they are incentivized to lower that bar as far as possible), none of the dollars go to open source maintainers - and it shouldn't in this case, because that would create a bigger problem.

However, the companies that use open source and benefit from it are another story. I love the community and love open source, but I find it absurd to see people from companies like IBM on this thread complaining about my responsiveness to the issue, whilst doing nothing to solve the real problem, which is a lack of funding. It's disheartening to see someone complain about me not spending time on this, and then actively trying to hurt my reputation or ask people to switch away from my projects, instead of caring if I get compensated equitably for my time, or if I'm struggling to pay my bills.

[jonschlinkert]

This was resolved in 3.0.3