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 prototype pollution for Hapi v16 #230

Closed
mcollina opened this issue Feb 15, 2018 · 10 comments
Closed

Fix prototype pollution for Hapi v16 #230

mcollina opened this issue Feb 15, 2018 · 10 comments
Assignees
Labels
security Issue with security impact

Comments

@mcollina
Copy link

As reported in https://hackerone.com/reports/310439, can we have the security fix backported to Hoek 4/Hapi v16 ASAP? Those are vulnerable, still supported, and highly used in production systems.

Snyk is currently flagging the remediation has "upgrade to hapi v17" which is not a good remediation strategy for this type of security fix (and have an extra cost).

@nlf
Copy link
Member

nlf commented Feb 15, 2018

I commented out of band, but it's worth mentioning here that actually exploiting this vulnerability in a hapi server requires that a user either neglect to validate user input entirely, or allow unknown keys on their payloads and pass one of those unvalidated/undervalidated payloads to one of merge, applyToDefault, or applyToDefaultWithShallow so this isn't as severe as it may sound on the surface.

That said, hoek@4.2.1 has been published with this fix backported.

@mcollina
Copy link
Author

Thanks for the release!

@dgonzalez
Copy link

dgonzalez commented Feb 15, 2018

On my opinion prototype pollution can lead into a form of remote code execution hence fixing it makes a lot of sense. I deal with a lot of code from the security point of view and input validation is often missing or incomplete (which leads into why injection is the number one on the OWASP top 10). Also I don't think it is a low severity bug.

I have attached a screenshot on the CVSS score calculation on the best case scenario given that it is exploitable via network and assuming that the application needs credentials to login (which might not even be the case).

screen shot 2018-02-15 at 19 28 00

I can see that the CVE has been reserved but not completed in here: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2018-3728 so I am not sure whether it will continue being a low.

@nlf
Copy link
Member

nlf commented Feb 15, 2018

It has been fixed.

To further discuss, while yes prototype pollution itself can lead to remote code execution this is a more difficult case wherein the attack must survive a JSON.parse. This means you can't pass in a function directly. This leaves you in a position where you could overwrite existing functions with a primitive value, causing a denial of service, but the likelihood of getting actual RCE is slim unless something somewhere in your request chain is running eval or similar on the parsed payload to convert a string representing a function into an actual function. That does not happen in this module, and as such the scope of that type of attack is outside of this finding.

@dgonzalez
Copy link

dgonzalez commented Feb 15, 2018 via email

@davedoesdev
Copy link

FYI @github is reporting 4.2.1 as vulnerable in its alerts. It says versions < 5.0.3 are vulnerable.

@nlf
Copy link
Member

nlf commented Apr 26, 2018

yeah, so i've heard. i have no idea where github pulls their data from but it's definitely wrong, nsp, the nodejs security WG, and even snyk all have correct data, yet @github is way behind despite the fact that advisory was written quite some time ago

@davedoesdev
Copy link

davedoesdev commented Apr 27, 2018

I think they're getting it from https://nvd.nist.gov/vuln/detail/CVE-2018-3728

Interestingly that references https://www.securityfocus.com/bid/103108 which has been updated correctly but the main CVE hasn't.

@davedoesdev
Copy link

davedoesdev commented May 1, 2018

FYI @github is marking 4.2.1 as fixed now. They were pretty responsive on email and turned it round in < 24 hours.

@lock
Copy link

lock bot commented Jan 9, 2020

This thread has been automatically locked due to inactivity. Please open a new issue for related bugs or questions following the new issue template instructions.

@lock lock bot locked as resolved and limited conversation to collaborators Jan 9, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
security Issue with security impact
Projects
None yet
Development

No branches or pull requests

5 participants