Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Security Fix for Prototype Pollution - huntr.dev #4

Merged
merged 2 commits into from
Nov 1, 2020

Conversation

huntr-helper
Copy link

https://huntr.dev/users/VitorLuizC has fixed the Prototype Pollution vulnerability 🔨. VitorLuizC 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 | 418sec#2
Vulnerability README | https://github.com/418sec/huntr/blob/master/bounties/npm/deep-get-set/1/README.md

User Comments:

📊 Metadata *

Bounty URL: https://www.huntr.dev/bounties/1-npm-deep-get-set

⚙️ Description *

On get and set function I've checked if key is __proto__, prototype or constructor and if does, it don't set or get the value. I've also added unit tests to check against this fix.

💻 Technical Description *

I created isSafeKey that receives key and returns a boolean value to indicate if it is safe to be getted or setted. isSafeKey was added to if conditions in both get and set iterations.

  • I've keep code-style.
  • I've used just ES5 functions, to support any older brower or Node.js version.

🐛 Proof of Concept (PoC) *

const deep = require('deep-get-set');

const obj = {};

deep.p = true;
deep(obj, '__proto__.hasOwnProperty', () => true);

const anyOtherObj = {};

anyOtherObj.hasOwnProperty('not its property');
//=> true;

🔥 Proof of Fix (PoF) *

const deep = require('deep-get-set');

const obj = {};

deep.p = true;
deep(obj, '__proto__.hasOwnProperty', () => true);

const anyOtherObj = {};

anyOtherObj.hasOwnProperty('not its property');
//=> false;

👍 User Acceptance Testing (UAT)

  • I've executed unit tests.
  • I've added unit tests to prove the fix.

@acstll
Copy link
Owner

acstll commented Sep 16, 2020

Thank you all, I'm gonna try and merge/release this fix later today!

@JamieSlome
Copy link

Hey @acstll, any updates on this?

@acstll acstll merged commit a127e65 into acstll:master Nov 1, 2020
@acstll
Copy link
Owner

acstll commented Nov 1, 2020

@JamieSlome I think I did it and it's published on npm

Sorry about the super long delay. I really don't have time for this. I'll be glad to hand over this package if needed.

Thanks again for the update.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants