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

Prevent prototype pollution chaining to code execution via _.template #4355

Merged

Conversation

alexbrasetvik
Copy link

@alexbrasetvik alexbrasetvik commented Jul 8, 2019

Prototype pollution is a common problem for Javascript applications.

This paper (PDF-link) is a reasonably comprehensive resource on the subject. It demonstrates that the problem is quite ubiquitous, and how it can turn into an unauthenticated RCE in Ghost, where the attack chain starts with a prototype pollution vulnerability in lodash.merge. Handlebars is used as the code execution gadget, but as Ghost also uses _.template it could have targeted that as well.

Lodash has had multiple prototype pollution vulnerabilities, including recently.

If _.template is ever invoked after the prototype has been polluted, an attacker can execute any Javascript, as sourceURL and variable are looked up via an options object and injected into the Javascript that gets executed. If the caller is not passing those options (which is the most common usage), then the values from the potentially polluted prototype would be used instead. Here's the smallest possible POC:

// If the prototype is polluted via some other vulnerability
> ({}).__proto__.sourceURL = '\nconsole.log("injected code here")'
'\nconsole.log("injected code here")'
// ... then arbitrary code can be injected
> _.template('hey')({})
injected code here
'hey'

I initially reached out via npm security, and got the feedback that this is not considered an issue. (In the thread with NPM security I also provided examples of how to weaponise this against real applications instead of just the repl example. I can provide additional examples privately.)

I think it's worth fixing, as the fix is simple. Applications that are susceptible to prototype pollution will still need fixing, but at least the commonly used _.template shouldn't provide easy code execution.

(I've signed the CLA.)

... and not via its prototype, as that enables chaining a prototype
pollution into arbitrary code execution.
@jdalton
Copy link
Member

jdalton commented Jul 8, 2019

Ah, I thought the one raised by npm was about the data object and not the options object. Adding guards to the options object is less risky in terms of a back compat concern so I'm up for this fix.

@alexbrasetvik
Copy link
Author

@jdalton Great, thanks for the quick response :) I adjusted the regexp to strip all [\r\n], instead of leaving lone \rs around.

lodash.js Outdated
('sourceURL' in options
? options.sourceURL
(hasOwnProperty.call(options, 'sourceURL')
? options.sourceURL.replace(/[\r\n]/g, ' ')
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you coerce options.sourceURL to a string. Something like (options.sourceURL + '').replace would do.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed :)

@jdalton
Copy link
Member

jdalton commented Jul 9, 2019

Awesome! Thank you so much @alexbrasetvik!

Update:

Published updated versions of lodash, lodash-es, lodash-amd, and lodash.template.

@jdalton jdalton merged commit 60eb517 into lodash:4.17.12-pre Jul 9, 2019
kobelb added a commit to kobelb/lodash that referenced this pull request Jul 9, 2019
@alexbrasetvik
Copy link
Author

Thanks for the quick turnaround! :)

@lodash lodash locked and limited conversation to collaborators Jul 17, 2019
@lodash lodash deleted a comment from alexbrasetvik Nov 16, 2021
@jdalton jdalton added issue bankruptcy Closing the issue/PR to start fresh and removed issue bankruptcy Closing the issue/PR to start fresh labels Sep 16, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging this pull request may close these issues.

None yet

2 participants