[BUG] Vulnerabilities Found in Micromatch and Braces

[MarioTeixeiraCx]

Hello @jonschlinkert,

I'm from the Checkmarx CxResearch group, and I've been trying to reach out regarding some vulnerabilities we found in Micromatch and Braces. Sorry for opening another issue, but last time, you closed without giving it a chance.

You told us to follow the security policy, and that's what we've been doing from the beginning. We tried several times to reach out to [email protected].

It has been over 90 days since our initial contact, so given our responsible disclosure policy, we intend to publicly disclose these vulnerabilities by the end of the week.

Thank you for your time.

Best regards,
Mário Teixeira

[jonschlinkert]

Hi @MarioTeixeiraCx, can you point out at least one library that implements micromatch or braces that is susceptible to the vulnerability so we can see how it's actually a vulnerability in the real world, and not just theoretical? Feel free to email that to me discretely. Then I'll get a patch pushed up right away, and then we'll update the changelog with details describing the vulnerability and how it can happen.

FWIW, I get probably 50 emails a day at that email address and the vast majority of them are people asking me to endorse something, or headhunters, etc. I'm sorry if I missed your emails.

[MarioTeixeiraCx]

Hi @jonschlinkert,

Thank you for your quick response here. I just sent you another email with the previous thread (the subject is "Vulnerabilities in Micromatch and Braces") since we prefer to treat it privately. I attached the report again and a real-world example.

Thank you,
Mário Teixeira

[MarioTeixeiraCx]

Hello again @jonschlinkert,

We're attempting to reach out to you once again to conclude the ongoing discussion, and we appreciate your understanding. We have correctly followed the security policy and tried contacting you via email multiple times, but we didn't receive any reply.

So, aligned with our responsible disclosure, we will soon proceed to request the CVEs.

Thank you for your comprehension.

Best regards,
Mário Teixeira

[mpassell]

@MarioTeixeiraCx Can you explain in more detail why the code in question is considered a vulnerability? Excuse my rather out-dated reference, but this seems a bit like pointing out that asking a DOM parser to parse a very big XML file takes a long time and consumes a fair amount of memory. Yes, you could switch to a SAX parser instead, but that involves a lot of extra work, it's not the fault of the DOM parser, and that isn't how it will typically be used. Seems rather similar to this...

[archanasharma3]

Ran a security scan and got the vulnerability issue -
I got this issue on index.js file on line 448 - if ((options && options.nobrace === true) || !/{.*}/.test(pattern)) {

CWE-1333 | Inefficient Regular Expression Complexity

The NPM package "micromatch" is vulnerable to Regular Expression Denial of Service (ReDoS). The vulnerability occurs in "micromatch.braces()" in "index.js" because the pattern ".*" will greedily match anything. By passing a malicious payload, the pattern matching will keep backtracking to the input while it doesn't find the closing bracket. As the input size increases, the consumption time will also increase until it causes the application to hang or slow down. This issue could be mitigated by using a safe pattern that won't start backtracking the regular expression due to its greedy matching.

I am on the latest version of micromatch 4.0.5 and this issue exists. Please take a look to resolve this.

[MarioTeixeiraCx]

Hello @mpassell,

Micromatch and Braces are intended to be used as libraries. Even though there is an expected use case for how the parser works, it is not guaranteed that the input from a certain implementation will exactly follow that. In a situation where the attacker could control this input, it's possible to exploit the vulnerabilities we reported because the DOM parser doesn’t handle memory properly or limit the processing of large data. We have also provided you with a “real-world” example of an application that uses the affected code and could be exploited by a malicious user.

Also, in agreement with @archanasharma3's reply.

Nonetheless, we would still prefer to discuss this privately, so we would appreciate it if you could answer our email or otherwise provide us with an alternative channel to reach out in private. In the meantime, we intend to proceed with the CVEs.

[RobinGiel]

could this pull request fix the issue ?

[MarioTeixeiraCx]

Hello @RobinGiel,

I've answered in the Pull Request that the issue still persists.

If you haven't requested CVEs yet, we can do it ourselves and keep them reserved for now. We wanted to confirm because we saw you mentioned CVE here: #247 (comment).

Thank you,

Best regards,
Mário Teixeira

[rmn183]

can we roll out new versions for above 2 with the fixes, any ETA please? thanks

[silverthornekevin]

Like rmn183 asked, is there going to be a fix for Braces and Micormatch in regards to the Checkmarx finding?

[MarioTeixeiraCx]

Hello @RobinGiel and @mpassell,

The CVEs are now public with the following IDs (micromatch and braces, respectively):

• CVE-2024-4067
• CVE-2024-4068.

As we said earlier, the PR #243 (comment) is not fixing the issue of CVE-2024-4067. We recommend a pattern that doesn't backtrack to the input, and there are tools such as this one that can help understand if a given regex is vulnerable. Additionally, the fixes should be present in a new version release to allow everyone to fix the issue by updating to that version.

Best regards,
Mário Teixeira

[mpassell]

@MarioTeixeiraCx I'm not sure why you at-mentioned me or Robin Giel. I'm not a contributor and I think Robin only contributed his attempted fix (sorry if that's incorrect). I just commented on this issue because I was frustrated at something being elevated to a "security" issue when it seems more like non-optimal code. It might seem like a small thing, but it puts a significant burden on the project maintainers and it means that those of us who are downstream of the project have to keep filing paperwork explaining why this really isn't a significant risk.

[paulmillr]

@MarioTeixeiraCx Why is every small shitty issue getting CVE and "npm audit" warnings these days? This really degrades the quality of "CVE"s overall. The Boy Who Cried Wolf sort of thing.

[jonschlinkert]

Why is every small shitty issue getting CVE and "npm audit" warnings these days? This really degrades the quality of "CVE"s overall. The Boy Who Cried Wolf sort of thing.

I get these issues all the time for things that can't even be a vulnerability unless you do it to yourself. Like regex in low level libraries that will never be accessible to a browser, unless you're letting users submit regular expressions in a web form that are just used by your application.

[jonschlinkert]

@MarioTeixeiraCx I asked for examples of how a real-world library would encounter these "vulnerabilities" and you never responding with an example.

[christian-kreuzberger-dtx]

those of us who are downstream of the project have to keep filing paperwork explaining why this really isn't a significant risk.

Hi! Paperwork-affected person here 😂 (looking at 4228 Dependents of micromatch, I'm probably not going to be the only one).

Personal Opinion: It seems it's not an actual risk, especially when those two package are only used by dev-dependencies (like jest/globals, changesets). When it comes to direct user input, I'm not so sure...

Anyway, it would be nice if we can confirm that someone is working on a fix/workaround to mitigate the "vulnerability"?

[jonschlinkert]

@christian-kreuzberger-dtx if you would be so kind as to show an example of the vulnerability and a real-world use case of why it's a vulnerability, I would greatly appreciate it.

Anyway, it would be nice if we can confirm that someone is working on a fix/workaround to mitigate the "vulnerability"?

It would be nice to confirm that it is one.

[coderaiser]

Here is example:

let line = '';

for (let i = 0; i < 100000000; i++)
    line += 'a';

console.time('vulnerable');
console.log(/{.*}/.test(line));
console.timeEnd('vulnerable');

console.time('still vulnerable');
console.log(/\{.*?\}/.test(line));
console.timeEnd('still vulnerable');

console.time('fixed');
console.log(line.startsWith('{') && line.endsWith('}'));
console.timeEnd('fixed');

And result:

[Mitsunee]

Who rated this 7.5? 330ms on a 100 MILLION character long input string is barely even a realworld performance improvement, and in neither case is this an actual security issue.

[doowb]

console.log(line.startsWith('{') && line.endsWith('}'));

You know that the regex checks for braces anywhere in the string, not just the start and end?

[paulmillr]

@MarioTeixeiraCx can you retract your CVE? It's useless.

[MarioTeixeiraCx]

Hi @jonschlinkert,

We have been following our responsible disclosure, which is also aligned with the general good practices and the procedure detailed in the Security Policy. As such, we had privately disclosed the details, including proof of the real-world use case you had requested.

We are still open to further clarification and have contacted you again on the previous email thread.

Best regards,
Mário Teixeira

[coderaiser]

You know that the regex checks for braces anywhere in the string, not just the start and end?

Is it really needed in this case?
I haven’t seen tests that passes braces in the middle of the line, so it not looks like real usecase.

Who rated this 7.5? 330ms on a 100 MILLION character long input string is barely even a realworld performance improvement, and in neither case is this an actual security issue.

Using RegExp here is slower in 10_000 times: 330ms vs 0.03ms. Do we really need it here?

[Mitsunee]

Is it really needed in this case?
I haven’t seen tests that passes braces in the middle of the line, so it not looks like real usecase.

This response shows that you have not even seen the README file of this repository. Under Matching Features there are multiple examples for brace expansion, all of which have braces in the middle of the string. Furthermore RegExp can be used to find multiple matches in the same input string.

Using RegExp here is slower in 10_000 times: 330ms vs 0.03ms. Do we really need it here?

See above.

---

I want to also again reiterate that this is not a security vulnerability and I would love to know where you expect malicious user input is ever sent to a library such as braces or micromatch. Rating this a security vulnerability with a HIGH rating wastes everyone's time for a proposed performance improvement which breaks features.

As for a proper solution that would actually retain functionally indexOf could be used to find the first/next instance of { after which slice and indexOf could be used to find the matching }. I suspect this is still significantly faster than the improved regex. If that's too fancy a simple for loop is also possible. Or just don't have a . in your pattern when [^}] is possible idk.

[coderaiser]

Here is documentation, you better fix it so it corresponded to what you want to say:

Do we actually need any kind of braces check, if nobraces passed? It should treat everything inside braces with braces included like literal characters thats what documentation says, no word about some strange additional checks.

What purpose to pass nobraces to brace library if it doesn’t support such option?

micromatch.braces = (pattern, options) => {
  if (typeof pattern !== 'string') throw new TypeError('Expected a string');
  if ((options && options.nobrace === true) || !/\{.*?\}/.test(pattern)) {
    return [pattern];
  }
  return braces(pattern, options);
};

[jonschlinkert]

I haven’t seen tests that passes braces in the middle of the line, so it not looks like real usecase.

echo one/{a,b,c}/two

[coderaiser]

We can use includes in this case:

micromatch.braces = (pattern, options) => {
  if (typeof pattern !== 'string') throw new TypeError('Expected a string');
- if ((options && options.nobrace === true) || !/\{.*?\}/.test(pattern)) {
+ if ((options && options.nobrace === true) || !pattern.includes('{')) {
    return [pattern];
  }
  return braces(pattern, options);
};

And it will be enough, if it doesn’t contain opening brace, what purpose to check any RegExp to search for a closing one?

[jonschlinkert]

And it will be enough, if it doesn’t contain opening brace, what purpose to check any RegExp to search for a closing one?

What if it does include a single one?

[BlueTux611]

For this issue proposed this pull request.

[coderaiser]

What if it does include a single one?

Would it be correct glob that worth parsing?

[kaizvn]

I'm just wondering that this is still a "vulnerable issue" or not? And how can we avoid getting these false alarm like this in the future? How can we appeal?

Update: I want to propose my solution here: the requirement of braces are existing in string pattern and the close brace should be after the open one.
So we may update the condition to:

   const openBraceIndex = pattern.indexOf('{');

   if((options && options.nobrace === true) || (openBraceIndex >= 0 && pattern.indexOf('}') > openBraceIndex) {
     //
   }
    

[coderaiser]

Looks like micromatch not maintained at all it hasn't updated for two years, better to use glob, if you use fast-glob, or globby. Or minimatch that was updated a couple months ago.

[paulmillr]

@MarioTeixeiraCx can you drop the “vulnerability” poc in my email?

[jonschlinkert]

Looks like micromatch not maintained at all it hasn't updated for two years, better to use glob, if you use fast-glob, or globby. Or minimatch that was updated a couple months ago.

@coderaiser try to keep it civil. If a micro library is changing and receiving patches all the time that usually means it's unstable or buggy.

I always fix real vulnerabilities immediately.

[coderaiser]

@paulmillr POC micromatch/braces#36 (comment)

[Experimental-products]

@jonschlinkert It may well be that there's no actual real-world functional exploit right now.

But we live in a world of innovation and a world of hackers. So tomorrow, someone may create a completely novel way to use micromatch that leaves their software with an accessible exploit. Or existing users may change their software in that way, leaving them vulnerable.

It would therefore be good to see the community coming together on an acceptable fix, and seeing that released soon for the benefit of everyone. Then we can get a beverage and snack of our choice and relax, knowing this noise is all in the past.

Thank you for your time and contributions!

[CodeByCalvin]

anyways will close it since braces fix is in and we just need to release it. micromatch will autofetch the update.

When do you expect the Braces release to come out?

[cathyyoung]

@paulmillr I noticed that the releases page shows 4.0.4 as the latest release from 2021, but the npm micromatch page shows 4.0.5 from 2022.

Our vulnerability scanner is currently flagging 4.0.5 since that's the npm version we have installed; could you confirm that a new micromatch release will be 4.0.6 even though there's no 4.0.5 on Github?