There is a patch that proposes to update the external library code for PageTriage. This code hasn't been updated since 2012 in the case of backbone.js, not sure about underscore.js offhand.
Creating this task to document the change and also ask if we need a security review on it.
| Subject | Repo | Branch | Lines +/- | |
|---|---|---|---|---|
| Update backbone.js and underscore.js | mediawiki/extensions/PageTriage | master | +4 K -2 K |
Change 666595 had a related patch set uploaded (by Kosta Harlan; owner: Ladsgroup):
[mediawiki/extensions/PageTriage@master] Update backbone.js and underscore.js
This is a case where the use of foreign-resources.yaml would be really useful for the auditability purpose...
Yeah. To be fair, the introduction of backbone/underscore libs in PageTriage preceded foreign-resources.yaml by ~8 years or so.
So, for now I suppose we could add foreign-resources.yaml based on the contents that are there, then look at doing this upgrade after a security review?
Yeah, I wasn't expecting a time machine :). But there's definitely value in doing this going forward, at least when we're upgrading libraries and/or making changes like this. Don't necessarily need to do it retrospectively (though if we can, and can trivially prove what commit/version it came from, there's definitely no harm in doing so, and sticking it as a parent patch).
As for what we're doing about security reviews for this upgrade, TBD :).
Hey @kostajh - we're looking at having an external vendor complete this review. Whether or not that happens, we'll plan to have an update for you within two weeks (2021-04-28).
Update: we plan to review the report deliverables for this and other vendor-performed reviews towards the end of next week (2021-07-29).
@kostajh, @Ladsgroup - We had one of our vendors (ROS) perform a security audit of the versions of backbone.js and underscore.js within the relevant gerrit change set. Here are their findings:
Vulnerability ID: WKM-005
Vulnerability type: Vulnerable software component
Threat level: critical
Description
The version of underscore.js targeted for upgrade is potentially susceptible to a known script-injection vulnerability.
Technical description
The review of the planned upgrade of underscore.js from version 1.3.1 to 1.12.0, described here, revealed that the target upgrade revision includes a known vulnerability: CVE-2021-23358. The proposed 1.12.0 patch set was confirmed to include the vulnerability.
underscore.js is a library that provides a collection of utility functions and abstractions for JavaScript development; it is a dependency of backbone.js. One of the features provided by underscore.js is a micro-template metalanguage that can be used to render content partials such as those that may be created by an application at run-time from JSON objects. In 2021, a vulnerability was identified related to the use of templates by applications: code injection within the DOM context of an application may be possible if an attacker can control how variable names are passed to templates. This is due to inadequate input validation and may present an elevated risk in contexts where there is increased trust of user-supplied content elements (and increased reliance on sanitization and validation of this content).
The fix is located here in the project source repository: https://github.com/jashkenas/underscore/blob/master/modules/ template.js#L71 .
The developers have fixed this by adding a validation check to the template variable values based on the following regular expression:
^\s*(\w|\$)+\s*$
This regular expression is used to verify that the variable is a sequence alphanumeric characters; whitespace or characters such as ' or " in a string will not match, resulting in a failure of the validation check.
Below is the definition of the regular expression to ensure the variable identifier is bare :
// In order to prevent third-party code injection through // `_.templateSettings.variable`, we test it against the following regular Findings 11 // expression. It is intentionally a bit more liberal than just matching valid // identifiers, but still prevents possible loopholes through defaults or // destructuring assignment. var bareIdentifier = /^\s*(\w|\$)+\s*$/;
And then later:
var argument = settings.variable; if (argument) { // Insure against third-party code injection. (CVE-2021-23358) if (!bareIdentifier.test(argument)) throw new Error( 'variable is not a bare identifier: ' + argument ); } else { // If a variable is not specified, place data values in local scope. source = 'with(obj||{}){\n' + source + '}\n'; argument = 'obj'; }
The patch set proposed as the upgrade target was reviewed and confirmed to not include the above fix.
Impact
The impact of this issue may be elevated due to the level of control that some Wikimedia deployments grant to untrusted users.
Recommendation
Target a more recent release of underscore.js for upgrade. Given the relationship between the projects, his may implicate backbone.js.
I updated underscore and backbone to newer version. FTR, when I made the patch, that was the most recent version.
Do you have a gerrit url handy? I'm just seeing the un-merged, old one: https://gerrit.wikimedia.org/r/666595
Change 666595 merged by jenkins-bot:
[mediawiki/extensions/PageTriage@master] Update backbone.js and underscore.js