fix: prototype pollution vulnerability in extend (CVE-2024-45435)

[andersk]

• Fixes [Bug]: Prototype Pollution Vulnerability Affecting chartist module, versions >=1.0.0 <=1.3.0 #1427.

https://nvd.nist.gov/vuln/detail/CVE-2024-45435
https://gist.github.com/tariqhawis/c67177164d3b7975210caddb25b60d62

[tariqhawis]

Hi @andersk ,

Thank you for your feedback regarding the report. I would like to highlight an additional prototype accessor: constructor.prototype. A potential injection could look like this:

extend ({}, JSON.parse('{"constructor":{"prototype":{"polluted":yes}}}'))"

Checking for the presence of constructor in the input should be sufficient.

Best,
Tariq

[andersk]

Checking for the presence of constructor in the input should be sufficient.

Nope, it’s not.

extend({}, {"hasOwnProperty": {"polluted": "yes"}});
console.log(Object.prototype.hasOwnProperty.polluted); // → yes

I’ve pushed a more complete fix.

[praveen-zensar]

Please merge this fix as it's a severity score is high.

[danielpalme]

I propose the following change for this line:
if (prop === "__proto__" || prop === "constructor" || (targetProto !== null && prop in targetProto)) {

See: https://codeql.github.com/codeql-query-help/javascript/js-prototype-pollution-utility/

[andersk]

Your change isn’t necessary because both __proto__ and constructor are already skipped by targetProto !== null && prop in targetProto.

[dangreen]

All usage cases are where first argument is empty object. We can rename extend to merge, remove first argument, and as target use Object.create(null).

I can fix it by myself later on next week

[mihaileu]

seems that @andersk might not be able to help here

[andersk]

No I agree in principle, but here’s the problem: Chartist.extend is part of the public API of this package, and if we bump the semver major version to remove it, that’s going to take years to propagate through the ecosystem…

[dangreen]

@andersk You should run "update storyshots" action manually in your fork. Then download artifacts and update screenshots in your branch.

[dangreen]

@andersk
put all images from https://github.com/andersk/chartist/actions/runs/14316237387/artifacts/2897050220
to https://github.com/andersk/chartist/tree/main/test/__image_snapshots__

[dangreen]

https://github.com/chartist-js/chartist/releases/tag/v1.3.1