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

fix: Resolve potential prototype polution exploit #217

Merged
merged 1 commit into from
Oct 20, 2022

Conversation

JSMike
Copy link

@JSMike JSMike commented Oct 20, 2022

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Oct 20, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: JSMike / name: Mike Cebrian (f4e48a2)

@JSMike JSMike closed this Oct 20, 2022
@JSMike JSMike reopened this Oct 20, 2022
@alexander-akait alexander-akait merged commit a93cf6f into webpack:v2.0.0-branch Oct 20, 2022
@alexander-akait
Copy link
Member

Thank you

@dingjiedanielyang-sec
Copy link

Hi Team,
Is this really a vulnerability as I was not able to really exploit it? Below is the POC code. I was not able to pollute the Prototype property of other objects.

const parser = require("./parseQuery.js")
let result = parser.parseQuery('?{a:"1",__proto__:{"isPolluted":true}}')
let result2 = parser.parseQuery('?{b:"3"}')
console.log(result)
console.log(result.isPolluted)        // True (The prototype of result has a new property isPolluted defined)
console.log(result2.isPolluted)       // Undefined (It should return true if the Prototype vulnerability exists )

@theJuan1112
Copy link

Hello,
I ran into this cve/issue from work and decided to look into it as I see that many packages depend on this and was considered a CRITICAL. I agree with the comment from @dingjiedanielyang-sec saying that there was no vulnerability in the first place, because every time parseQuery runs it is creating a NEW empty/null object and populating it according to the provided string , i.e. you can't pollute something that has "nothing" in the first place. The "fix" pretty much did nothing. This maybe will have being a different story if it started with some other non-null object.

What could be a security concern is how this new object will be handled, as you can technically create one with arbitrary properties that could be malicious. However this will be up to the applications/other-packages using this as a dependency to properly "sanitize" the query/string provided and object.

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

Successfully merging this pull request may close these issues.

4 participants