Security Fix for Prototype Pollution - huntr.dev

[huntr-helper]

https://huntr.dev/users/alromh87 has fixed the Prototype Pollution vulnerability 🔨. alromh87 has been awarded $25 for fixing the vulnerability through the huntr bug bounty program 💵. Think you could fix a vulnerability like this?

Get involved at https://huntr.dev/

Q | A Version Affected | ALL Bug Fix | YES Original Pull Request | https://github.com/418sec/object-path/pull/2 Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/npm/object-path/1/README.md

User Comments:

📊 Metadata *

object-path is a tiny JavaScript utility to access deep properties using a path (for Node and the Browser).

Bounty URL: https://www.huntr.dev/bounties/1-npm-object-path

⚙️ Description *

Prototype Pollution refers to the ability to inject properties into existing JavaScript language construct prototypes, such as objects. JavaScript allows all Object attributes to be altered, including their magical attributes such as proto, constructor and prototype. An attacker manipulates these attributes to overwrite, or pollute, a JavaScript application object prototype of the base object by injecting other values. Properties on the Object.prototype are then inherited by all the JavaScript objects through the prototype chain.

💻 Technical Description *

Fixed by avoiding setting magical attributes.

🐛 Proof of Concept (PoC) *

1. Install the package(npm i object-path), run the below code:

var objectPath = require("object-path");

var obj = {};
console.log("Before: " + obj.polluted);
objectPath.set(obj, "__proto__.polluted", "pwned!"); 
console.log("After: " + obj.polluted);

2. Outputs pwned!.

🔥 Proof of Fix (PoF) *

After fix execution obj prototype is unmodified, undefined is returned

👍 User Acceptance Testing (UAT)

After fix functionality is unafected

[JamieSlome]

@mariocasciaro - would love to get your thoughts on this fix and see if we can get it merged.

Let me know if you have any questions! 🍰

[mariocasciaro]

Hi @JamieSlome and @alromh87 thanks for this PR and for starting the conversation. By default object-path does not set inherited properties, so this vulnerability does not exist in the default operating mode. In fact, the proof of concept you are proposing is not accurate. The last line should read console.log("After: " + Object.prototype.polluted) and will demonstrate that the polluted object is indeed NOT set in the prototype, as expected.

However, I believe that the vulnerability exists when a new object-path instance with the includeInheritedProps: true options is used, or alternatively when the withInheritedProps alternate default instance is used.

So the fix is slightly more complicated than this. If you still want to submit the PR please amend it with the following:

• The function 'set()' should throw an exception saying that "For security reasons, object magic properties cannot be set".
• The function 'set()' should throw the above exception only when the "withInheritedProps" option is set
• Please add some unit tests to verify the code

I'll wait a couple of hours for the amendments, then, since it's a security issue that we are talking about, I'll proceed to fix it myself.

Thanks again for reporting.

[mariocasciaro]

Actually @JamieSlome @alromh87 , I'll just proceed to fix it myself immediately since it's a security issue.