qs: [invalid CVE]: “Property Injection” in the function merge() CVE-2021-44907

Update - just got a reply from my CVE denial request and it looks like they approved the denial. The CVE status should be updated within a few hours. I’m not sure how long it takes GitHub’s advisory database to update its status, but I imagine within 24 hours or so.

I’m confused; why is there a CVE that wasn’t responsibly disclosed first? Filing security issues publicly is insanely irresponsible, and wildly insecure.

Someone who knows how to file a CVE should absolutely know how to follow a security policy.

Just an update on this - I have received nothing more from MITRE other than the automated confirmation that they’ve received my rejection request and will review it. Honestly, I don’t know how timely of a response to expect, or whether to expect one at all…

I’ve submitted a Rejection request to the following form: https://cveform.mitre.org/

Looks like this should be resolved.

Please let me know if any tooling is complaining about this invalid CVE.

Seems weird to me that a CVE can just be reported like this without disclosing it to the developer first, and now there’s no clear way to declare it as a false positive. Now millions of people have to land here to discover it isn’t an issue, wasting a collective bucket of hours.

Someone just pointed this thread out to me as Ramda goes through the same frustrating process. The issue was opened by the same user who started this one. I can’t tell from a glance through this whether or not qs is actually subject to the reported problem. Ramda isn’t. It took a long battle to get Veracode to withdraw this, and now we have to fight Mitre too. I never even got an automated acknowledgement of my rejection request when I sent it more than two weeks ago.

I’m glad we’re not alone. But there’s something very, very wrong with the process here.

Another update - I have tried several times to get a response from MITRE and nothing has come back. I’m not sure what to do at this point.

…merge() (…) allows to assign properties on an array in the query. In case of any property being assigned a value the array is converted to an object containing these properties. Essentially, this means that the property whose expected type is Array always has to be checked with Array.isArray() by the user, which may not be obvious to the user and can cause unexpected behavior. While this seems intentional, this behavior is not stressed in documentation.

This is absolutely expected and documented behaviour.

From the README:

https://github.com/ljharb/qs/blob/542a5c7ff88d7229efa2e22c7c8a7d69375f5e72/README.md?plain=1#L270-L275

A couple of simple examples: https://jsfiddle.net/1s7pq93z/1/ https://jsfiddle.net/65jxksay/

You don’t even need the above examples to demonstrate that parsed values may not be an array:

qs.parse('arr=a&arr=b').arr; // ['a', 'b'] - array, great! 👍🏻
qs.parse('arr=a').arr;       // 'a'        - not an array! 😱
qs.parse('arr[a]=a').arr;    // {a: 'a'}   - not an array! 😨
qs.parse('').arr;            // undefined  - not an array! 🤦🏼‍♂️

I don’t think this library ever guaranteed returning arrays. Why would this suddenly become a security vulnerability?!

@Marynk (or anyone with the power), please close this CVE at the source and save 1000s of other developers the pain of even looking at this issue.

Side note: the second example above doesn’t fail because the expected arr array is converted to an object, it fails because it is calling .map() on the returned query object.arr isn’t even referenced.

const parsed = Qs.parse("arr=a&arr=b&arr=c&arr.map=true", {allowDots: true});
const edited = parsed.map(el => el+"test"); // Should be parsed.arr.map(...) 

@samandar-boymurodov then it seems pretty obvious that it hasn’t been yet

Please let me know if any tooling is complaining about this invalid CVE.

@ljharb WhiteSource is still complaining during SAST scan: https://www.whitesourcesoftware.com/vulnerability-database/CVE-2021-44907