[ CVE-2022-37600 ] / Prototype Pollution found in clone.js

[secdevlpr26]

Prototype pollution vulnerability in function clone in clone.js in remy nodemon 2.0.8 via the attr variable in clone.js.

The prototype pollution vulnerability can be mitigated with several best practices described here: https://learn.snyk.io/lessons/prototype-pollution/javascript/

[remy]

And how are you exploiting this code exactly?

[remy]

More importantly, can I ask where you copied this text from?

[rezoled]

It seems like a critical CVE was opened for this issue:
https://ossindex.sonatype.org/vulnerability/CVE-2022-37600?component-type=npm&component-name=nodemon&utm_source=dependency-check&utm_medium=integration&utm_content=7.2.1
When checking for dependency issues it fails the most recent version of nodemon (2.0.20) because of this ticket.

[remy]

Yeah, because of this ticket not because there's actually an exposed vuln... ffs.

[remy]

Yeah, so this is a completely false report that can't be replicated. Here's the code in question: https://jsbin.com/rimurur/2/edit?console

What's extra rubbish is that sonartype has accepted the cve without any proof of concept.

What a waste of everyone's time.

[Garrestocles]

Hi @remy,

Sorry about the consternation this is causing. The Security Research team at Sonatype looked into the vulnerability and it does appear that @Supraja9726 reported a real prototype pollution issue. As you've pointed out, because the report was a little vague, we wrote out a PoC for it to make sure. Assuming you want it, what would be the best way for us to send it to you?

[remy]

@Garrestocles you can send it to remy@remysharp.com

[remy]

Sent a reply, the PoC doesn't actually use nodemon to pollute the chain.

[huineng]

It's cumbersome to follow up on all these CVE's, on top of that this also reported as critical.
I would appreciate it to know if this is going to be solved somehow, or if i can suppress it.

Thanks

[remy]

@huineng I'm fairly sure this is a false and wrongly been report. The PoC was this:

    const nodemon = require("nodemon");
    const target = {};

    nodemon({

        a: target, 

        b: __proto__.constructor.prototype.toString = function() {

            console.log("toString is polluted") 

    });
    const newTarget = {};
    newTarget.toString();

Which, for the keen eyed of you, can see that the pollution happens outside and regardless of nodemon. i.e. this is the same:

    const target = {};

    {

        a: target, 

        b: __proto__.constructor.prototype.toString = function() {

            console.log("toString is polluted") 

    }
    const newTarget = {};
    newTarget.toString();

I'm just waiting on a new PoC (I'm guessing @Garrestocles is on US time and/or is busy with real work :) )

[huineng]

thanks for your quick reply, i agree , that example can apply to any function.

[Garrestocles]

We looked at it again and you're right, @remy. There's that getOwnPropertyNames call in your merge method that's preventing the pollution of the prototype chain. We'll go ahead and remove it from our data.

So, I know other organizations also look at Github issues for vulnerability advisories, so you might see this showing up from other vendors in the future. Were I you, I'd remove the CVE from the title of this ticket, and that might save you from future headaches if they haven't picked it up already. I see that this CVE ID in mitre/NVD is still listed as reserved, so I assume it hasn't actually been reported yet. Unless you see something we're missing here, @Supraja9726, would you mark this CVE as rejected in whatever CNA you were using, please?

[secdevlpr26]

Hello,

All the reports are based on the research work of my colleague (you can find her paper's link below) and I am reporting them here as per her analysis and records.

https://dl.acm.org/doi/pdf/10.1145/3488932.3497769 - This is the published paper with the Github link to her static analysis tool. The mentioned code has been flagged potentially vulnerable by the static analysis tool
Thanks

[remy]

@Supraja9726 right, but the problem is that you created a full CVE based on "potentially" when in fact the reality was that it was not possible.

This means there's a fault in the static analysis (likely that it's not thorough enough) and there's a fault in your process of follow up.

Currently this process, in my opinion, is irresponsible and needs to be rectified as I've seen the same thing happen to other maintainers having the burnden of having to prove there's no vulnerability.

[secdevlpr26]

Please refer to the below link for the PoC:
https://jsbin.com/qecehotoqo/edit?js,console

This does pollute the target object (although the global level object is not affected in this case) which can still be a potential danger to the places in the code where this object has been used.

For what it's worth, you can place controls on this target object such that the proto properties are not copied.
Thanks

[remy]

@Supraja9726 that's cute, except it's not my code. This is the clone function, last touched in 2015: https://github.com/remy/nodemon/blob/main/lib/utils/clone.js

So this begs the question, what on earth was your source for you to go off an (wrongly) create a cve?