LGTM.com - fail to detect guard performed in a function

[bcaudan]

trafficstars

Description of the false positive

From the working example on js/prototype-pollution-utility doc:

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

an alert is raised if the hasOwnProperty check is performed in a function:

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

function hasOwn(obj, key) {
    return obj.hasOwnProperty(key)
}

URL to the alert on the project page on LGTM.com

https://lgtm.com/projects/g/bcaudan/codeql-issue/rev/pr-14dbb16de38d18a9611f25683b3916a6748a375f

[edoardopirovano]

Indeed, this looks like a limitation of our analysis, many thanks for reporting this. As I understand it, we look for local calls to hasOwnProperty but we don't use our interprocedural dataflow in that sanitizer, so calls in a helper function aren't observed. I'll leave it to the JavaScript team to prioritise this false positive against other work.

cc. @erik-krogh since it looks like you most recently worked on this part of our analysis.

[erik-krogh]

Yes, that is a limitation of our analysis.

We already have some support for sanitizers that have been moved into a function, but we had limited that to functions with just 1 parameter.
(We tend to only add support for features after we've seen in the real world).

I've tried to generalize it a bit here: https://github.com/github/codeql/pull/10046.
That should fix this FP.