Problem/Motivation
yarn audit is showing one vulnerability on 10.1.x, and multiple vulnerabilities on lower branches.
Steps to reproduce
cd core
rm -rf node_modules
yarn install
yarn audit
Proposed resolution
yarn upgrade
yarn vendor-update
yarn build
yarn audit
There are no changes to built assets or production dependencies with the current approach, so no un-minification to audit them is required.
Remaining tasks
A Nightwatch dependency has a critical vulnerability on 9.x that is not addressed by upgrades allowed by the current constraints:
yarn audit v1.22.17
───────────────┬──────────────────────────────────────────────────────────────┐
│ critical │ flat vulnerable to Prototype Pollution │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ flat │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=5.0.1 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ nightwatch │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ nightwatch > mocha > yargs-unparser > flat │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1085318 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high │ minimatch ReDoS vulnerability │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ minimatch │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=3.0.5 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ nightwatch │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ nightwatch > minimatch │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1085778 │
└───────────────┴──────────────────────────────────────────────────────────────┘
┌───────────────┬──────────────────────────────────────────────────────────────┐
│ high │ minimatch ReDoS vulnerability │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Package │ minimatch │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Patched in │ >=3.0.5 │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Dependency of │ nightwatch │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ Path │ nightwatch > mocha > minimatch │
├───────────────┼──────────────────────────────────────────────────────────────┤
│ More info │ https://www.npmjs.com/advisories/1085778 │
└───────────────┴──────────────────────────────────────────────────────────────┘
3 vulnerabilities found - Packages audited: 1184
Severity: 2 High | 1 Critical
✨ Done in 1.43s.
User interface changes
API changes
Data model changes
Release notes snippet
JavaScript dependencies have been updated to their latest patch releases to fix security issues.
Comments
Comment #2
xjmComment #3
xjmComment #4
xjmTagging "Needs followup" for the issues that aren't addressed within current constraints on 9.x (namely the critical Nightwatch issue).
Comment #5
xjmMost of the changes to 9.5.x seem to be no-op code cleanups. However, 9.4.x has more substantive changes to the following files:
core/misc/date.jscore/misc/jquery.cookie.shim.jscore/misc/message.jscore/modules/ckeditor5/js/ckeditor5.admin.jscore/modules/ckeditor5/js/ckeditor5.jscore/modules/system/js/system.modules.jscore/modules/views_ui/js/views_ui.listing.jsComment #6
xjmThe 9.4.x patch failure seems to be from:
No idea what to do with that. 🤷♀️
Comment #7
xjmComment #8
xjmComment #9
xjm@longwave suggested that we could upgrade the vulnerable dependencies only by removing it from the
yarn.lockand rerunningyarn.install. I tried removing both json5 and debug, then runningyarn install. This still leaves:...but that same vulnerability is also present with the patch from #3, so we'd need to change constraints to fix it.
Attached shows the manual removal I made to
yarn.lock, and the resultant update forjson5only in the patch.Comment #10
xjmForgot to attach the manual removal diff; doing so here.
Repeating this on 9.5.x.
yarn auditof HEAD gives:On 9.5.x this saves a whole lot of random little changes to the built assets, +1 for this approach.
Comment #11
xjmFor 9.4.x:
Much cleaner update, no built asset changes and therefore hopefully no
yarn checkfailure.Comment #13
longwaveAdding credit for Spokje who brought this issue to my attention privately a few days ago, and who demonstrated the same solution as #9.
Comment #14
xjmComment #15
spokjeRegarding:
This seems to be caused by an incorrect CVE based on a correct CVE from 2018(!), so there isn't an actual vulnerability, (that was in 2.6.8 and fixed in 2.6.8, which we're using) at least that's what I understand from https://github.com/debug-js/debug/issues/924 and https://github.com/expressjs/express/issues/5088#issuecomment-1378028914
All we can done here, I reckon, is to wait until the incorrect CVE has been reviewed and changed/revoked.
Comment #16
xjmComment #17
xjmSo let's ignore
debugfor now (as well as a handful of other vulns on 9..x that aren't cleaned up with simple lockfile updates) and focus on gettin these for now-simple patches in? And have followups to address the remainingyarn auditissues when either our constraints allow it or upstream makes it easier.Thanks!
Comment #18
spokjeCVE on
debughas been updated, warning inyarn audithas disappeared.Comment #19
spokjeSo there's a lot going on in this issue with multiple patches for multiple core versions.
Let's see if we can go through all of them in a series of comments.
For
10.0.x-devand10.1.x-devpatchyarn-3332447-9-10.0.x.patchfrom #9 results in a "clean"yarn audit:Comment #20
spokjeOn
9.5.x-devapplying patchyarn-3332447-9-9.5.x.patchfrom #10 gives us:That's 2 vulnerabilities less than without the patch:
Comment #21
spokjeOn
9.4.x-devapplying patchyarn-3332447-11-9.4.x.patchfrom #11 gives us:That's 15 vulnerabilities less than without the patch:
Comment #22
spokjeThe now-simple patches reduce the amount of vulnerabilities detected with
yarn audit.On 10.0.x and 10.1.x they reduce them to 0.
For me this is a RTBC.
Comment #23
longwaveRemoving frontend framework manager review tag, I don't think it's needed now we are no longer making changes to built assets, and added a short release note snippet.
Also removing "needs followup" as I don't believe we can do anything about the remaining Nightwatch issues in 9.x without a major version upgrade, which is not possible if we also want to retain backward compatibility.
Committed and pushed c13e8ec365 to 10.1.x and 9130250b65 to 10.0.x.
Committed and pushed 82f4da1540 to 9.5.x.
Committed and pushed f08ff26754 to 9.4.x. Thanks!
Comment #24
xjmComment #25
xjmThis does belong in the 10.1.x release notes since the dependency updates there are much broader than the patch releases' updates; however, for now, I've left it to the normal note of:
(The second half of that isn't true yet, but will be before release.)