[CVE-2022-37601]/Prototype pollution found in parseQuery.js

[secdevlpr26]

Prototype pollution vulnerability in function parseQuery in parseQuery.js in webpack loader-utils 2.0.0 via the name variable in parseQuery.js.

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

[alexander-akait]

Please migrate to 3 version, 2 version is deprecated and doesn't supported, thank you

[JSMike]

From the error report The issue is here:
https://github.com/webpack/loader-utils/blob/v2.0.2/lib/parseQuery.js#L29

Because of the following result[name] assignments. This report seems to want const result = {}; to be const result = Object.create(null); if you're going to manipulate an object the way you are to avoid a prototype pollution exploit.

Per the reporting tool this was reported to you in: #212

[alexander-akait]

@JSMike Thank you, published https://github.com/webpack/loader-utils/releases/tag/v2.0.3

[matek075]

@JSMike Is this issue resolved in the new version or I have to overwrite pareQuery.js ?

[G-Rath]

@w3bdesign if Dependabot is still showing the vulnerability as open in your codebase but giving that message, then its because you've got multiple versions of loader-utils in the dependency tree, some of which are vulnerable but currently dependabot only sees the "first" one.

@alexander-akait I'm sorry I have to ask, but any chance of getting this backported to 1.x? It looks like this is commonly pulled in by babel-loader who have moved away from it as a dependency in v9 but that meant dropping webpack v4 support, and we're sadly still locked into v4 for a while longer due to setups like @rails/webpacker that are no longer maintained but require enough time and budget to move off that we can't just do it for all of our apps (😢)

As always I'm happy to do anything I can to make it less work for you 🙂

Turns out babel-loader was able to upgrade to v2 in a v8 patch before they released v9, so the v2 backport is sufficient 🎉

However resolve-url-loader is currently on v1 for their v3, so a backport would be useful for that...

[ashkulz]

@alexander-akait this also affects us, the dependency tree is

@vue/cli-service@5.0.8
  @vue/vue-loader@15.10.0
    loader-utils@1.4.0

There's no branch for 1.x else I'd have submitted a PR 😅 Hopefully the upstream issue vuejs/vue-cli#7323 is actioned soon 🤞

[ashkulz]

Thanks for the quick release of 1.4.1, @alexander-akait 🎉

[danielhaim1]

🎉

[jdgregson]

Is there a working PoC for this issue? This same pattern of dest[foo] = src[foo] has been marked as prototype pollution on many different repositories recently. However, after much research, I have not found the pattern to be inherently vulnerable, and a good answer here shows that it is not.

If there is a way to exploit this I'd love to know, but I feel that this is very likely a false positive.

[theJuan1112]

I also agree that this is most likely a false positive. I went into a little more details on a comment in the "fix" they did for this. The only thing I can come up that could lead to security issues is how the object created will be handled, but that is up to the apps/packages using this as a dependency to figure it out in their specific use case.

Also I checked the person who reported this and they are pretty much spamming a bunch of projects alleging these types of vulnerabilities but providing no context and wasting everybody's time. In this issue this person alleges he is using a static code analyzer to find this "vulnerabilities" from a research paper. I just reported this person for spam.

[alexander-akait]

I agree, databases pay such people money not for quality but for quantity, but it has no value. I think that databases should be more stringent in exploit reporting. This is not the first time I've encountered this and I have to fix nothing a lot of time just to avoid "vulnerabilities messages".

[alexander-akait]

A minimally reproducible example should be provided, firstly internally to fix a problem, then publicly

[theJuan1112]

I actually just watched the presentation for the research paper on this and the presenter specially noted that their tool/analyzer does NOT take into account context, which that is the MAIN point of doing proper security research/disclosure. This is just so bad/annoying because you are getting reports of a CRITICAL in your apps, especially for such widely used dependencies, which you are forced to check. Oh well, time to move on.

[jdgregson]

After some research over the last few months I have found that the original research is correct in finding that dest[foo] = src[foo] is inherently vulnerable to prototype pollution, though there is a push to call this variant "prototype poisoning". This vulnerability does not pollute the global object prototype. Rather, it poisons the prototype of the dest object:

const src = JSON.parse('{"__proto__": {"polluted": true}}');
const dest = {};
Object.keys(src).forEach((key) => {dest[key] = src[key]})
window.polluted; // undefined
dest.polluted; // true

This can allow an attacker to bypass filters in some cases by adding hidden keys to the dest object.

I am not sure what the original vulnerability was in this case, but I found that someone mentioned the the exploit code would be a query like {a:"1",__proto__:{"isPolluted":true}}.

I looked into it and found that loader-utils was vulnerable as the above payload resulted in prototype poisoning. However, loader-utils simply passed such payloads to json5. I investigated json5 and found that it was the cause of the prototype poisoning, and it was fixed as CVE-2022-46175 .

Whether or not the vulnerability was actually impactful given the context is another question.

[JeremyRH]

I think we agree this "prototype poisoning" is not the same as prototype pollution. It requires the code using parseQuery.js to check iterable own keys on result for "protected" keys then assume this check was thorough enough. It does increase the chance for vulnerabilities but should not be classified as "critical" IMO.
How do we go about adding a new common vulnerability called "prototype poisoning"?