-
Notifications
You must be signed in to change notification settings - Fork 991
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Policy: Addressing CVEs raised by old versions of yargs #1839
Comments
@pjsharpe07 I can't bring myself to give you a hard no on back-porting, for the benefit of gulp 😆 Let's try the approach of having this catch all thread, and we can decide on a case by case basis how to approach things -- then I can close CVE PRs and direct people to this conversation. |
It looks like the |
@DullReferenceException this prototype pollution issue was already addressed in [email protected]. CC: @lirantal for visibility, we'd initially released the fix as |
OK, thank you. I guess the issue is just that |
On the Snyk side, the database should be up to date now with 5.0.1 including the fix. |
new issue detective for |
Refs: #2029 |
@bote795 patched in |
The LTS nodejs (14.17.6) includes yargs 14.2.3 as a dependency, which still pulls in a vulnerable ansi-regex version. Does yargs 14 need to be updated so nodejs can update it? |
@woehr [email protected] is pinned on Best approach might be trying to update the dependency in Node.js, what's yargs used for in Node? (I'm guessing it comes in with another dep?). |
It gets pulled in by npm (through libnpx). |
@Qix- you're going to hate me ... but how hard would pulling a patch all the way back to Kind of cool we've managed to cause an issue in the platform itself right? |
I'm getting a security issue in Snyk for the latest version of [email protected] for "string-width": "^4.2.0". This might be a known issue already but just wanted to make sure. It looks like "string-width" bumped their dependency of ansi-regex in "5.0.1" |
@joeylgutierrez [email protected] pulls in [email protected] which is fixed: └─┬ yargs@17.2.1
├─┬ cliui@7.0.4
│ ├── string-width@4.2.3 deduped
│ ├─┬ strip-ansi@6.0.1
│ │ └── ansi-regex@5.0.1
│ └─┬ wrap-ansi@7.0.0
│ ├─┬ ansi-styles@4.3.0
│ │ └─┬ color-convert@2.0.1
│ │ └── color-name@1.1.4
│ ├── string-width@4.2.3 deduped
│ └── strip-ansi@6.0.1 deduped
├── escalade@3.1.1
├── get-caller-file@2.0.5
├── require-directory@2.1.1
├─┬ string-width@4.2.3
│ ├── emoji-regex@8.0.0
│ ├── is-fullwidth-code-point@3.0.0
│ └── strip-ansi@6.0.1 deduped
├── y18n@5.0.8
└── yargs-parser@20.2.9 If you have a local |
So I haven't forgotten about the request to backport - I've been giving it some thought. I can of course backport, but I'd rather not, for two reasons. Sorry for the long winded answer, I don't expect everyone to read it. I know that some of us come off a bit crude when responding to these sorts of issues so I want to explain, fully, my standpoint when it comes to trivial fixes, backports, the CVE process and these vulnerabilities. Firstly, the reported ReDos vulnerabilities affect only specific cases when very large, unsanitized inputs are passed to the regex. Of course in some cases the processing time is exponential, but it doesn't affect all inputs - in the cases where the affected branch is activated, short inputs will see no significant slowdowns. There is currently a sort-of battle being fought that a few maintainers have spoken with me about, but so far hasn't really been talked about publicly. A lot of our packages are getting hit with "high" CVE vulnerability reports because a certain security firm that has jumped onto the scene within the last year or two keeps reporting them one-by-one, seemingly to milk bounty programs of money. Of course the intent here is speculative and reflects my own suspicions first and foremost, but I can say definitively that I'm not alone in going down this line of thinking. The reason why they score "high" is two-fold: firstly, CVE scoring is widely considered outdated and poorly designed. It does not take into account that vulnerable code is usually behind layers of abstractions, or that certain "vulnerabilities" are not really vulnerabilities and instead are misuses from the downstream application code. As an example (raised years ago by a different researcher), Chalk had a vulnerability report that unsanitized inputs to Chalk could include "harmful" escape sequences that controlled hardware. Well, yeah. Of course. We're not going to mangle your input - Chalk is purely additive. The same "vulnerability" would be there if you simply wrote to stdout/err. There was a bit of a scuffle over email about it and ultimately we had to simply ignore the researcher entirely after some rude remarks were made toward the project. This sort of behavior is pretty uncommon in my personal experience, but the wave of ReDos vulnerabilities seems to be a new variant of this behavior. The researchers in question have been polite, but relentless, and instead of addressing all of them at once and assigning a reasonable CVE score, the reports are instead made on websites where thousands of collective dollars are being rewarded for very small amounts of work. Every ReDos reported so far could have been detected by a trivial automated test. None of this money goes to actually benefit the project or guard against regressions, or to even support the developers who maintain those libraries. It feels really slimy, and it awards trivial amounts of work with lot of money whilst simultaneously damaging the good-faith open source projects. Not to mention, taking a LOT of time responding to the resulting issues (most of which are duplicates), requests (like this one) for backports, or other strange requests people seem to think they need fulfilled. Further, we've seen users migrate away from our libraries after these are filed as they perceive the code to be insecure and full of bugs - which simply isn't the case. Further, a few times now I've personally requested that a CVE is not filed for such vulnerabilities since they are very edge case and can be fixed trivially. The amount of impact or harm is basically zero, and I've never personally heard of any of these libraries causing outages of any kind, much less from ReDos attacks. Since the CVE score generally states that the ReDos is "remotely exploitable" (i.e. over a network) since the library could be used in a network application, that automatically sets the score to a very high medium. Then the other few points usually alloted to the score push it up to the "high" every single time. In theory, that makes every single piece of code in the Node.js ecosystem (if not just about everywhere else) "remotely exploitable" and thus every vulnerability could be seen as "high". CVE scores are broken, in my opinion, and the researchers in question appear to be using them to make money. Again, this is my interpretation of the situation. I have nothing but respect for responsible disclosure processes and security research, but this feels like a blatant money-making grab. Should we be implementing automated tests? Absolutely. Are ReDos vulnerabilities actually vulnerabilities? Of course. Should we patch them and push updates? Yes*. The asterisk there brings me to my next point. Small, edge-case vulnerabilities are everywhere. Since these reports keep coming up, we're going to keep getting requests to backport. A few times now, we've also been asked to backport full features to old major versions of the codebases, because we backported hotfixes shortly prior. This is not a precedent I want to set. It very much increases the surface area to break legacy software, it increases the OSS maintainence overhead, it produces a lot of noise, and it creates a lot of previously-nonexistent surface area for human error to wreak havoc. This has happened a few times in the past where a bad publish broke a bunch of people (millions of people) and we had to put out fires and then respond to the tsunami of issues caused by people facing aggressive caches and whatnot. At this point, I'd rather just encourage people to upgrade. Sorry for the novel! |
Rather than opening a new issue for a CVE, please start a conversation here.
We can decide on a case by case basis if it makes sense to attempt a back port, or to help the upstream dependency update yargs.
The text was updated successfully, but these errors were encountered: