Repository navigation
[GHSA-vfj7-8cjw-p6xm] braces vulnerable to stack-exhaustion denial of service through deeply nested patterns - #10132
Conversation
|
+1 to this correction. Some additional evidence: Maintainer position: The braces maintainer has addressed this report directly (micromatch/braces#70 (comment)): the nesting depth needed to overflow the stack is only reachable near the default 10,000-character limit, and the existing, documented Reproduction (braces 3.0.3, Node.js 24.21.0, 10 fresh processes each, pattern
The failure only occurs close to the input-length cap, is non-deterministic below it, and is a catchable Reachability: micromatch's matching APIs ( As published, the advisory produces a high-severity finding with no remediation path for a large part of the npm ecosystem, while the exploit precondition — letting untrusted users supply ~9,000-character glob patterns — is already a misuse of the library. Marking no version as affected (or withdrawing the advisory) seems appropriate. |
|
Removing the The proper way to get this addressed is to submit a request to the CNA to withdraw the advisory |
|
(Once someone submits the request, please post here to keep people informed so that they don't receive duplicates.) |
|
up |
Are you trying to bump the thread or are you saying that you've made the request to remove the advisory? |
just saving to get notified on updates, sry |
A tip, clicking the Subscribe button below will keep you updated. |
|
The reported behavior is quite specific: an input that is still below the library's documented That distinction matters particularly because braces explicitly describes itself as safer for applications receiving aggressive or malicious brace patterns: https://github.com/micromatch/braces/blob/c57d89e50aefa70a2f7463fdc785923321f1e62c/README.md:
If the position is instead that callers are responsible for ensuring brace patterns are trusted or have shallow nesting, that is a defensible API contract, but it would be different from what the current documentation suggests and should be stated explicitly. I'm not an experienced Node.js programmer but why doesn't the author simply accept his own responsibility to catch There is also an important distinction between a vulnerability in the library and exploitability in every downstream application. braces is obviously not a network server by itself. A remotely exploitable DoS requires an application to pass attacker-controlled patterns into it. Many projects only use braces with repository-controlled globs and may therefore be effectively not affected. That is a good argument for contextualizing the CVSS rating and for downstream projects to document a ‘not affected’ determination. It is not an argument that uncontrolled recursion in the library cannot constitute a vulnerability at all. |
@sanmai-NL The CVE is based around the fact that a
To resolve the "vulnerability" as described by the CVE one of these would suffice:
|
|
@mrgrain Can you rephrase your last sentence please? |
There's no standard that requires to not catch an exception in this case. As I wrote,
Callers do have to worry for sure. Interesting also, how input sanitization of tainted data is presented as a use case for the library, while its maintainer @jonschklinkert, in defense against this CVE filing, claims:
This is a weakness in itself, in particular given how
Of all the nuances stated in the thread, no sound argument has been presented that this isn't a vulnerability. Maybe its severity is overrated, maybe it's safe to say the exploitation conditions are unlikely, true.
https://csrc.nist.gov/glossary/term/vulnerability
https://csrc.nist.gov/glossary/term/weakness The essential issue is that the library is not secure by design (and little can be done about it, perhaps) but presented as such.
Documenting which exceptions are thrown does seem the best remediation. There are plenty of simple mitigations and remediations that the author could have applied straight away, rather then letting this escalate into security pipeline issues for so many people and organizations.
Outside Node.js, raising exceptions is for exceptional conditions. In this case, invalid input data is provided and a normal error-as-value return would not cause crashing Node.js. |
Does this help?
I think people have different views on what "crashing Node.js" means. Note how JavaScript (and Node.js) calls these |
That makes sense, I used GitHub's advisory improvement suggestion form to make this change and it spat out this diff. Since it's a branch on this repository I also can't edit it now... |
|
For anyone that thinks this is a legitimate vulnerability, I have bad news... The following also causes the $ node -e "require('braces')(JSON.parse('null'))"
[...]
TypeError: Cannot read properties of null (reading 'length')
[...]anyone up for submitting a CVE for it? 🙃 |
|
If the author took care to address the previous one, this one couldn't be filed anymore. Maybe you find comfort in clowning about on this CVE but having it disappear won't solve this package's/Node.js common quality problems. |
Excuse my joking around, but it was the most concise way to make my point.
Neither will this individual CVE address the ecosystem's quality problems. Moreover, the ecosystem has a solution for this: every package interacting with the network worth it's salt will catch any error for you and prevent your application from crashing. On the other hand, noisy CVEs like this one waste time, energy, attention, and money. The result is that developers have a distaste for the security ecosystem, which makes it all the more difficult for the ecosystem as a whole to deal with "real", or at very least more relevant/pressing, security issues. To add something more constructive to the discussion:
And neither has there been a sound argument presented that this is a vulnerability.
I'm all for documenting this, but documenting it and then asking users to upgrade to the latest version just so they get the updated documentation seems pointless to me.
I will say that I disagree with the author of this comment. It should be possible (in theory) to fix the reported bug and avoid the
From the evidence you, @sanmai-NL, provided,
And do what exactly? |
No individual CVE will address any ecosystems quality problem, so that point is moot. Interacting with the network should be broadened to accepting tainted data. This package,
While I agree with the critical sentiment, you don't provide evidence for this broad claim, and this case can be countered quite easily. The
The onus is on you, to support your argument that a vulnerability report, which details the claim, which was then accepted by at least one CVE-issuing authority, is indeed invalid. You and others here raise a few points, but should a CVE be retracted just after some backlash? I think there's a better chance the severity rating will be adjusted.
And why so? A large part of digital security is pointless to technically oriented people. All the process controls and certifications, all the posturing by vendors. Why should your and some other GitHubians viewpoint prevail in this larger context?
It's a security improvement since it improves security by design, and it's a vulnerability remediation as long as the
That's not entirely true. The maintainer claims very broadly:
... and then provides one example of a mitigation that's implemented. You see that as limitative, but the wording is ambiguous at the very least.
As I wrote earlier, returning errors as values instead of propagating exceptions makes for more robust systems. |
I've been replying to emails all past week from people asking me why this |
Ok
Again, sure. No CVE needed.
Fair enough, though I'm afraid it would be impossible for me to provide evidence for sentiment...
If you really believe it's OK for some random person to get a random CVE assigned to random projects and the best course of action is to waste the developer's time by making them do a release just to make the CVE machine happy then you and I are so fundamentally in disagreement that I don't see how any amount of discussion could lead to fruitful results.
Why is that? Just because the CVE already exists as a result of a CVE-farming individual who convinced a numbering authority with no skin in the game to publish one? In any case, let me spell out my argument in more detail:
The only assumption I made, as far as I can tell, is that we're considering "safe" usage of the
Again, I can turn it around, why should "their" point of view prevail. I can give you one concrete reason why "our" (sidenote: it's bold of you to assume which "side" I'm on) opinion should at least matter: in the end "we" are the ones that have to deal with it. To be more constructive, let my clarify why it seems pointless to me (under the assumption that the vulnerability has some merit and the viable fix is to update the documentation):
I ask you, what did upgrading achieve? Lastly, like it or not but you are just as much a "GitHubians" as the rest of us in the context of this discussion 🙂
Once again I don't disagree that such improvements are worthwhile. However, neither are sufficient grounds for a vulnerability advisory as far as I'm concerned.
I can concede the wording is ambiguous. Again in this case, I'm all for improving the wording but creating a CVE for ambiguous wording seems beyond reasonable to me...
Since you're not willing to get concrete I'll attack a strawman instead. Returning More generally, if you don't like the fact that package throw exceptions, don't use them. If you don't like JavaScript, don't use it. |
Uhm, vulnerability -> CVE. Misleading docs -> vulnerabilities .
Then why did you express it?
Interesting, your use of random qualifier. What are you trying to say exactly? Should only accredited people and officially designated products be involved in this? The developer himself wastes his users time by falsely marketing a package. Why not just be honest and let the users look elsewhere if they want to be hand-held?
Uhm, yes. You say it as if you find it unfair. But who promised to be fair to you and people who're annoyed by this too, in this thread?
I think the technicalities have been discussed to death already. Nobody disagrees. The problem is, the package makes the claim that developers no longer have to worry about the weakness we see here, because his package is ‘safer‘. You can reason about the exploitation risk, etc., but packages marketed misleadingly to the many developers who know much less about security than you do are a real problem. You don't seem to accept that.
And why is that unthinkable? I'm trying to tell you, try to take a different perspective for once. I agree, from a somewhat similar perspective as yours, that it's silly to come to think of all the risks due to uncaught exceptions in the Node.js ecosystem. But that doesn't mean there's no problem. Driving without a seatbelt is stupid, still, people do it.
Have you checked who's actually more in charge in our world? PhD students and researchers, or big vendors and platforms?
I see from your position and arguments which side you're on, on the premise that these sides exist. Not so bold really. ‘Street garbage collectors now demand, that people shouldn't produce so much waste, since it's them who have to deal with it.’ I hope you see the point now.
I think this is one of your weaker argumentation lines. Of course, nobody should change the version of a dependency without assessing it. A one line
Yeah, true. I hope some of us, like I do here, can take a broader perspective even if unpopular. Not just ‘meh, security risk low, lazy CVE filing, do not like maintenance work’.
This one line I think summarizes our discussion and positions very well. I think the broader view is worthwhile: many consumers (developers) use this dependency, and may very well be misled because of the false marketing, and signalling this through a CVE is useful. The work this encumbers the
Your claim I'm not willing to do something is unwarranted, and I have been concrete in mentioning the common name of a commonly known design pattern. I believe you honestly did not understand yet from my comment alone. Here's an example library that helps implement this pattern: https://www.typescript-result.dev/. You're a bit inaccurate here. The unchecked, undocumented, unhandled exception throwing antipattern specifically affects Node.js, not JavaScript engines in general, when used for CLI and server applications, and those who don't use conventional TypeScript or other, bare bones techniques in JavaScript. |
For the first one, sure(-ish). Under the premise that a documentation change would suffice in the case of this CVE, this doesn't apply. For the second one, No. Consider a function that states it always returns a value greater than 0 but actually always returns a value greater than 1. This is misleading, but I cannot see any possible vulnerability arising from it.
Why not? I'm very sorry to tell you but the topic of vulnerabilities is not purely factual. Whether something is a vulnerability, bug, or intended behavior is very much subjective. Hence, I don't see a reason to reject other types of subjective arguments outright.
My main point is that the opinion of the maintainer of the package should weight a hell of a lot more than some private GitHub account with, to the extend I was able to find, no credible history of vulnerability reports that is submitting LLM generated security reports of dubious quality.
We can be honest about that but last I checked (including in the very comment containing this quote) this is not grounds for a CVE.
That doesn't mean the onus isn't on you.
Please explain how such developers are (negatively) affected by the behavior described in this CVE.
It matters not to me who is in charge. In a functional system developers are involved because, again, they have to fix the problem in the end. If they choose the abandon the CVE system it breaks down regardless of who is "in charge".
I did not say/imply it is unthinkable, rather that it is comical/nonsensical. What perspective exactly do you want me to take?
Why is it OK to expect people to thoroughly review and understand dependency updates when it is not OK to expect people to understand the same dependency and put a
This argument is off-topic but if you must: Note I'm not a maintainer of Again I ask, what is the broader perspective I should take?
I'm sorry to tell you but "errors as values" has a much broader meaning than the individual example you gave. The one I sketched is(/was?) common in C. Go has errors as distinct values, but in a different style. The example you gave can be seen in e.g. Rust. Hence my provocation. I agree both the Go and Rust examples have benefits. However, I will point out that they still allow you to bypass them in ways that are just as detrimental as uncaught exceptions, if not more so in these particular languages. But I don't really see how any of this is relevant to the discussion. The package explicitly uses error throwing in its API so I really don't understand how one could argue that throwing errors is inherently wrong in this situation - this whole argument thread seems off-topic to me.
It's unclear to me how, e.g., Browser-based JavaScript fares any better. |
|
@sanmai-NL, I put your comments into an AI detector and your entire responses are AI generated. >90%. Just FYI to the rest of the group. Given that almost all of the arguments for the CVE here are AI-generated, I will summarize this as a problem of explosive complexity: we will only ever be able to guard against a limited range of scenarios. THIS IS A FACT. This applies to a backtracking NFA (like V8), a pure linear-time DFA, or a hybrid JIT compiler. If the regex engines can't even prevent this, how would we? Globs and brace patterns are regular expressions. While it's true that globs impose certain limitations on regular expressions, those glob-specific limitations themselves unfortunately do not stop exploding complexity. You can think of it as a "state-space explosion problem", where brace nesting is only one of many possible ways to explode the state-space. Brace nesting happens to be a highly visible way to create large structural trees during parsing, so it's an obvious way to create a state-space explosion, but state-space explosions happen from many other combinations of wildcards, repetitions, and (especially) negations. In other words, any user could easily trigger a catastrophic state-space explosion using a variety of basic regex features, even if the pattern is perfectly flat and completely devoid of nested braces:
In other words, once a patch to limit brace nesting is implemented, AI will just move onto another obvious state-space exploding pattern to create the next CVE. One might argue that we should still patch this because, "at least it will guard against brace depth". But my counter argument is that any bad actor can easily get around the limitations with many other patterns that cause explosions in state-space. So the patch to limit brace depth would do nothing for the library or users, while compensating the people who created the bullshit CVE. And given that the security researchers intentionally picked a low-hanging-fruit pattern to exploit, they are certainly aware of this. The only thing being exploited here is the community, and Snyk is the attacker. What say ye, ChatGPT? |
|
@ArianeBouchardConformit (and friends): I filed a dispute with MITRE (ID 2122324) on Sunday (NZT) about They contacted me yesterday asking
so at this point I've gotten more response from them than I have the CNA which is technically an improvement... |
|
@G-Rath Thanks a bunch for your involvement! I guess mine will likely get closed as a duplicate then. |
braces の上流のメンテナは、この advisory の修正版を出さない方針を示している (github/advisory-database#10132 のコメント)。braces は devDependencies の markdownlint-cli2 と semantic-release が micromatch 経由で使うだけで、渡す glob の パターンは自分の設定とステージ済みのファイル名なので、信頼できない入力を受けない。 - pnpm-workspace.yaml の auditConfig.ignoreGhsas に足し、理由と Issue をコメントに書く - docs/vulnerability-handling.md に、advisory を無視するときの方針と、 修正版の公開を人が確かめることを書く - kaizen: worktree の隔離セッションで複合コマンドが拒否された再発を、既存の学びに追記する Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…) (#673) * fix(deps): 修正版が出ない braces の GHSA-vfj7-8cjw-p6xm を pnpm audit で無視する(#576) braces の上流のメンテナは、この advisory の修正版を出さない方針を示している (github/advisory-database#10132 のコメント)。braces は devDependencies の markdownlint-cli2 と semantic-release が micromatch 経由で使うだけで、渡す glob の パターンは自分の設定とステージ済みのファイル名なので、信頼できない入力を受けない。 - pnpm-workspace.yaml の auditConfig.ignoreGhsas に足し、理由と Issue をコメントに書く - docs/vulnerability-handling.md に、advisory を無視するときの方針と、 修正版の公開を人が確かめることを書く - kaizen: worktree の隔離セッションで複合コマンドが拒否された再発を、既存の学びに追記する Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(deps): 無視した GHSA を dependabot-alert-issue の dismiss にも置き、パターンの出所を書き直す PR #673 のレビューの指摘への対応である。 - .config/skills/shoji9x9/skills.yml の dependabot-alert-issue.dismiss に GHSA-vfj7-8cjw-p6xm を 置く。Dependabot alerts に切り替えた後も、同じ advisory を却下の候補にするため - docs/vulnerability-handling.md に、ignoreGhsas と dismiss を同じ変更で足し外しすることを書く - pnpm-workspace.yaml のコメントに、CI の markdownlint-cli2 と semantic-release も設定ファイルの パターンを使うことを書く Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(deps): 無視の理由を条件の形で書き、dismiss を外すときに alert を開き直すことを書く PR #673 の 2 回目のレビューの指摘への対応である。 - pnpm-workspace.yaml: 上流の方針を「修正版が公開されるまで無視する」という条件の形に直す。 fork の PR が設定ファイルを書き換えても、止まるのはその PR の CI だけであることを書く - skills.yml: Dismissal comment を、誰が何を使い、何を渡すかが分かる文に直す - docs/vulnerability-handling.md: dismiss から外しても却下した alert は open に戻らないので、 GitHub で開き直すことを書く Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(deps): 無視を外す条件に依存が使わなくなった場合を足し、Dismissal comment を参照にする PR #673 の 3 回目のレビューの指摘への対応である。 - pnpm-workspace.yaml と docs/vulnerability-handling.md: 修正版が公開されたときに加えて、 どの依存もそのパッケージを使わなくなったときにも外すと書く。比喩の語を言い換える - skills.yml: Dismissal comment に理由を書き写さず、#576 と ignoreGhsas のコメントを参照する - .kaizen: 学びの見出しと提案の最初の行を、名詞を「の」で連ねない文に直す Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(deps): 無視の判断を「影響を許容する」にそろえ、無視した GHSA を audit の報告に添える PR #673 の 4 回目のレビューの指摘への対応である。 - pnpm-workspace.yaml: 設定ファイルの変更はレビューとマージを通ることと、fork の PR で 止まるのはその PR の CI だけで、その影響を許容することを書く - skills.yml と docs: reason の tolerable_risk に合わせて「影響を許容できる」と書く - docs/vulnerability-handling.md: 無視する advisory の取り決めを独立した節にし、 dependabot-core#15904 が解決するまでの手順と分ける - pnpm-audit-alert-issue: ignoreGhsas の advisory は pnpm audit に出ないので、 報告に無視している GHSA の一覧と件数を添える Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> * fix(deps): 無視した GHSA の報告を最後の手順に移し、パターンの出所に commit-types.ts を足す PR #673 の 5 回目のレビューの指摘への対応である。 - pnpm-audit-alert-issue: 無視している GHSA の一覧と件数を、findings の件数に関わらず 最後の報告に添える手順 9 を足す。一覧は pnpm-workspace.yaml の ignoreGhsas から読む - pnpm-workspace.yaml: semantic-release の releaseRules の元の commit-types.ts を、 パターンの出所に足す - skills.yml: Dismissal comment の Issue の参照を、alert の画面で開ける URL にする Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
Any word on the CVE dispute? |
上流のメンテナはこの advisory のパッチを出さないと述べている (github/advisory-database#10132 のコメント)。braces は devDependencies の markdownlint-cli2 だけが micromatch 経由で使い、渡すパターンは自前の設定と ステージ済みのファイル名だけなので、影響を許容する。 - pnpm-workspace.yaml: auditConfig.ignoreGhsas に理由と Issue 参照つきで追加 - skills.yml: Dependabot alerts 向けに dependabot-alert-issue.dismiss へ同じ GHSA を追加 - docs/dependency-policy.md: advisory を無視するときの方針と一覧を追加 Refs #147 Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
@asrichesson None on my side. Nothing whatsoever. I've submitted a follow-up on the request, but it's possible they're silently ignoring mine in favour of @G-Rath 's request... EDIT: I just realized there was a required field I hadn't filled in. And yet it allowed me to submit the form and led me to believe all was in order. What terrible UI. Anyway, I filled in the missing field just now. Possibly the wait for my request begins now. Sorry about that. |
|
It seems to me image-size had a similar issue, CVE considered pointless but causing tooling alerts. Apparently, they "fixed" this by migrating to another repository provider than GitHub. And now, the offending CVEs are no more creeping back in, it seems. No idea what that other repository provider does differently for that. |
|
Migrating to another hosting like Codeberg works temporarily imo. CVEs will be opened regardless of where you host your code. The same is valid with forks: you can fork and republish as many times as you want, but the CVE will catch up eventually. |
|
Do you have any updates @ArianeBouchardConformit? |
|
@Serhansolo None whatsoever. @G-Rath Anything on your side? |
|
I have successfully gotten CVE-2026-97058 rejected (including on VulnCheck who finally replied - ironically they replied on my braces dispute email), and have emailed GitHub about that to have them withdraw their advisory. For the other two there's been no update - it sounds like VulnCheck are not engaging with MITRE either, but I have been chasing them both. |
|
@G-Rath Thank you for your involvement. It's wild though that the VulnCheck vulnerability disappeared without a trace. I would have expected it to stay documented as a rejected item. I hope this will push the system towards an improvement in the end... |
|
I realized that GH is a dedicated CNA so forwards my dispute of CVE-2026-97058 on to them and they withdrew it from their database within the hour, so I've now forwarded them my disputes of all three advisories in the hopes they choose to withdraw them independently. |
|
Really appreciate your persistence on this @G-Rath - hopefully this gets propagated out and we can move on. |
Updates
Comments
No version of
bracesis affected by this. Observe the following two facts: 1) ARangeErrorin Node.js1 is catchable. 2) ThebracesAPI may already throw errors. Therefore, on untrusted inputs, developers usingbracesshould already be catching errors. Thus, theRangeErrorwould also be caught. This invalidates the advisory's claim that this error "terminate[s] the Node.js process".Footnotes
The only JavaScript runtime considered by the advisory itself. ↩