JS: Add 'prototype pollution in utility function' query

[asgerf]

Adds a query that looks for recursive merge functions that are susceptible to prototype pollution. It doesn't check for user-controlled inputs.

For example, it flags this function as vulnerable:

function merge(dst, src) {
    for (let key in src) {
        if (!src.hasOwnProperty(key)) continue;
        if (isObject(dst[key])) {
            merge(dst[key], src[key]);
        } else {
            dst[key] = src[key];
        }
    }
}

It looks for a property enumeration (the for-in loop above) and a dynamic property write (dst[key] = src[key] above), where three flow paths can be found:

• From key to the base of the dynamic property write
• From key to the property name of the dynamic property write
• From src[key] to the right-hand side fo the dynamic property write

It's relatively expensive as there are a lot of sources and sinks, and the graph pruning does not take into account that we require all three paths to exist. For example, if the base of a dynamic property write is definitely unreachable from a source, it might still try to find paths to the property name and RHS of that property write, even though these paths will never result in an alert.

The latest evaluation shows the first serious conflict I've seen between wall-clock time and tuple counts/DPMs, and I'd like to run a few more evaluations to determine which one tells the truth.

[erik-krogh]

What about the FP rate?

You get quite a few results just on nightly.slugs, so I worry that the results will be noisy once deployed on LGTM.

[asgerf]

There's a FP in jQuery due to our imprecise treatment of overloading in general. The rest are TPs as far as I can tell (although whether any user-controlled inputs flow there remains TBD).

[asgerf]

Re-running the evaluation shows the wall clock timing was a fluke. +1 for tuple counts/DPM.

[esbena]

Amazing! I am floored by how elegant and precise this turned out to be.
I do not think any of my comments require a new evaluation, well done.

I am a little surprised, but happy, that I can not find any places that explicitly requires the recursive merge call. My expectation for the query was that the recursive call was a key requirement that had to be explicit.

[asgerf]

Added a change note, and rebased to fix change note conflict.

I also changed the CWE numbers. Actually I'd like to move both the prototype pollution queries to CWE-471 (modification of assumed-immutable data) but I think I'll do that in another PR.

[esbena]

Excellent.
Ping @mchammer01 for a doc review

Two meta tasks:

• have you pinged maintainers or our security team about the (relevant) newly discovered vulnerabilities?
• will you update the tracking issues for the 10 CVEs that inspired this query?

[mchammer01]

@asgerf - this LGTM.
I have made a few minor comments and leave it up to you to decide whether they are worth addressing. There is a typo (in in) which needs fixed,
Hope this helps!

[mchammer01]

Suggesting: is usually reachable through the proto and constructor.prototype special properties.

[asgerf]

Hm, again it's fairly technical and I think it's easier to understand the other way.

[asgerf]

Thanks for the review @mchammer01

[mchammer01]

Thanks for the updates @asgerf