Coordinated Disclosure Timeline
- 10/19/2020: Report sent to Anton Kosyakov anton.kosyakov@typefox.io
- 10/20/2020: Report sent to project leads Sven Efftinge sven.efftinge@typefox.io and Marc Dumais marc.dumais@ericsson.com
- 10/21/2020: Report sent to Florent Benoit florent@redhat.com
- 10/28/2020: Proposed fix shared with GHSL
- 10/30/2020: Fix is merged
Summary
Prototype pollution in mergeContents and parseConfigurationData functions.
Product
https://www.npmjs.com/package/@theia/plugin-ext
Tested Version
Latest commit
Details
The package contains a merge function for recursively merging two configuration objects. It does not check for problematic properties like __proto__, and hence it is vulnerable to prototype pollution. Specifically, if the target parameter to this function is under the control of an attacker, they can supply a specially crafted object that causes the function to overwrite properties on Object.prototype or add new properties to it. These properties are then inherited by every object in the program.
Here is an example, in which we assume that payload is provided by an attacker, and isAdmin is a property that is later on used to check whether a user is authorized to perform some sensitive operation:
const ConfigurationModel = require('@theia/plugin-ext/lib/plugin/preferences/configuration').ConfigurationModel;
var payload = JSON.parse('{"__proto__":{"isAdmin": true}}');
new ConfigurationModel().merge(new ConfigurationModel(payload));
After this code has run, Object.prototype.isAdmin is true. But Object.prototype is on the prototype chain of most objects, so we also have {}.isAdmin == true, and indeed u.isAdmin == true for most other objects u, including, perhaps, objects used to represent users.
We haven’t been able to trace the provenance of the target parameter all the way back to its source, but looks to us like it may come from a configuration file. If that is true and a user’s IDE is set up to automatically import configuration files in projects that are opened in the IDE, then an attacker can exploit the vulnerability by putting a crafted configuration file into an otherwise innocuous open-source project.
In addition, parseConfigurationData also seems to be vulnerable to prototype pollution: if data contains a property named __proto__.isAdmin, the function will overwrite the Object.prototype.isAdmin property with the value of that property.
Impact
Information Disclosure, Escalation of Privileges
Credit
This issue was discovered and reported by GitHub team member @max-schaefer (Max Schaefer).
Contact
You can contact the GHSL team at securitylab@github.com, please include GHSL-2020-201 in any communication regarding this issue.