hapijs / hoek

Node utilities shared among the extended hapi universe
Other
481 stars 171 forks source link

Security Fix for Prototype Pollution - huntr.dev #363

Closed huntr-helper closed 3 years ago

huntr-helper commented 3 years ago

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/hoek/pull/1 Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/npm/@hapi/hoek/1/README.md

User Comments:

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-npm-%40hapi%2Fhoek

⚙️ Description *

hoek package is vulnerable to prototype pollution issue

💻 Technical Description *

Fixed by adding missing magical attributes, to filter. - if (key === '__proto__' || + if (key === '__proto__' || key === 'constructor' || key === 'prototype' ||

🐛 Proof of Concept (PoC) *

  1. Install the package(npm i hoek), run the below code:
var utils = require('hoek');
var src = {};
let object = utils.merge({}, JSON.parse('{ "myProperty": "a", "prototype" : { "isAdmin" : true } }'));
console.log(object.prototype.isAdmin);

Outputs: true.

hoekPOC

🔥 Proof of Fix (PoF) *

After fix object is unmodified

hoekPOF

👍 User Acceptance Testing (UAT)

After fix functionality is unafected

devinivy commented 3 years ago

@huntr-helper I realize you are a bot, but hopefully a human behind the scenes will find this.

Here is our security policy, which is also linked while filing an issue: https://github.com/hapijs/hoek/security/policy. For the safety of our users and to work within this organization's process it would be greatly appreciated to respect our wishes here. By this organization's standards this does not constitute responsible disclosure. Please send an email to security@hapi.dev if you find a security issue, or even a potential security issue, and we will happily work with you to come to a resolution. This of course applies to any public disclosure (i.e. via offering a bounty) without coordination with the hapi organization.

Fortunately in this case, there does not seem to be a security issue exhibited here. The proof of concept does not demonstrate an instance of prototype pollution because it doesn't indicate that the object prototype was actually mutated or reassigned. Hoek does protect from typical attacks against constructor because it only allows merging into objects.

However, if you do see an issue with prototype pollution in hoek please reach out to security@hapi.dev rather than continuing the conversation here.

Marsup commented 3 years ago

More discussions in https://github.com/hapijs/hoek/issues/362 as well.