denial of service and may lead to remote code execution vulnerability

[mdeknowis]

According to https://app-eu.whitesourcesoftware.com/Wss/WSS.html#!securityVulnerability;id=CVE-2020-28275 there is a

Prototype pollution vulnerability in 'cache-base' versions 0.7.0 through 4.0.0 allows attacker to cause a denial of service and may lead to remote code execution.

in

cache-base/index.js

Line 71 in e4d50b7

this.emit('set', key, ...rest);

There seems to be a CVE already assigned: https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-28275

Any chance to get this fixed?

[doowb]

Please provide some code to demonstrate/reproduce the issue you're having.
The links that you provided either don't contain any information or require signing into another service.

[BLoeT]

I think like others we only get the notification that there is a problem with this line 71 described in mdeknowis post.
The cve link works.

Whitesource: https://www.whitesourcesoftware.com/vulnerability-database/
there you can search for cve-2020-28275

But there are no more informations :(

[doowb]

Thanks @BLoeT.
When searching whitesource, I found the cve and they have a section with "PoC Code", however, that code does not demonstrate a security issue (the underlying library that handles "set" takes into account keys with __proto__, so the Object prototype is not modified).

Since the PoC Code doesn't actually show checking another object to see if the prototype has been modified, this shouldn't be a valid issue:

const Cache = require('cache-base');
const cache = new Cache();
const anotherObject = {};

cache.set('__proto__.polluted', 'true');
console.log(anotherObject.polluted);
// undefined

If this was a valid security issue, then anotherObject.polluted would be 'true'.

//cc @whitesource @rarkins how do we get this addressed in the whitesource database?

[danielelkabes]

Hi Brian (@doowb) we are trying to reach you and your team for the past two months, please check your mail and spam, there you can find a detail report, we can discuss this issue and figure out together what should be our next steps.

@BLoeT at this time you can see the details about this vulnerability here - https://www.whitesourcesoftware.com/vulnerability-database/CVE-2020-28275 . and here https://cve.mitre.org/cgi-bin/cvename.cgi?name=CVE-2020-28275

Thanks,

[rarkins]

@danielelkabes please see the comment above. The underlying set-value package does proto checking: https://github.com/jonschlinkert/set-value/blob/master/index.js

[doowb]

@danielelkabes I found your email from today and responded.

I never received the original email with a report, so I'm not able to speak to anything other than what's in the linked CVE. If there is more, I'm happy to have a discussion so we can create a reproducible test case and provide a fix.

@rarkins thank you for pointing to set-value (I just realized I didn't actually link to it 😄 )

[kachkaev]

It'd be great if a potential patch could be backported to older versions, including v1. Our project refers to cache-base@1.0.1 via chokidar v3, which is in turm coming from pretty recent babel and watchpack versions. 🙏

[danielelkabes]

Hi Brian (@doowb) after our additional discussions and review regarding this issue, we have agreed that there is no vulnerability here, and that you are indeed sanitizing the key value correctly within set-value package dependency.

Thanks for your assistance in resolving this, it is much appreciated!

[sokra]

Here is a repro which allows to pollute the prototype:

const Cache = require("cache-base");
const c = new Cache("cache");

const key = "__proto__"; // <- This need to be user controlled
const entry = c.get(key);
entry.hello = "polluted";

console.log({}.hello);

[snipem]

Hi Brian (@doowb) after our additional discussions and review regarding this issue, we have agreed that there is no vulnerability here, and that you are indeed sanitizing the key value correctly within set-value package dependency.

Thanks for your assistance in resolving this, it is much appreciated!

@danielelkabes Are you with Whitesource? Can we expect that you revoke the CVE and will not alert any further findings with Whitesource code checks?

[rarkins]

@sokra that prototype pollution appears to come from get-value. Repro: https://runkit.com/embed/ndj8wjcvv55q

Both v3 and v4 of cache-base depend on get-value@^3.0.1 so any fix published there (e.g. get-value@3.0.2) would then make this library non-vulnerable.

@danielelkabes will assess whether to update the description of the existing CVE or if it's more appropriate to issue a new one

[rarkins]

@snipem our comments crossed paths. There is a vulnerability remaining in this library although it's possible the existing CVE will be rejected and a new one issued for get-value instead. And yes, @danielelkabes is a security researcher at WhiteSource

[doowb]

Thank you @danielelkabes for looking into this further.
Thank you @sokra for the additional PoC when getting a value.
@rarkins I agree that this is something different, should be discussed separately, and patching any issues in dependencies is a better approach.

I'm going to close this issue and we can discuss get-value there.

[sokra]

Note there are a few more related (security) issues:

• new Cache("__proto__") sets the prototype of the instance, which removes all methods and set/get etc throw errors.
• similar problems with new Cache("constructor") or new Cache("set"), etc.
• c.get("hasOwnProperty") should probably not return the function on Object.prototype
• c.has("hasOwnProperty") should probably not return true for the function on Object.prototype
• some older cache-base versions allow to omit prop and set all entries directly on the Cache instance, which allows to c.del("__proto__.set"), c.del("constructor.prototype.set") to break all cache classes.
• c.set("xxx", {}); c.set("xxx", { __proto__: ... }) allows to modify the prototype of the object, which could break stuff
• c.set("hasOwnProperty", 1) and c.set("__proto__", 1) crashes. see this line (id can be a bad value)

[snipem]

@rarkins Wow that was quick. Our on-premise installation removed the vulnerability just minutes after our discussion. Thanks.

[doowb]

@sokra I agree with you that there are some other bugs/issues. I don't believe all of those are necessarily security related due to the difference in developer/implementor vs user supplied inputs.

Also, PRs are welcome with fixes for these bugs ;)

[corydorning]

any update on resolving this? still being reported via npm

[rarkins]

I'm not sure which DB npm uses but the CVE has been rejected officially: https://nvd.nist.gov/vuln/detail/CVE-2020-28275

I also don't see it here: https://www.npmjs.com/advisories