JavaScript: Restricting isSource predicate leads to more alerts

[max-schaefer]

I have the following test file for the UnvalidatedDynamicMethodCall query:

var express = require('express');
var app = express();

var actions = {
    play(data) {
        // ...
    },
    pause(data) {
        // ...
    }
}

app.get('/perform/:action/:payload', function(req, res) {
    if (actions.hasOwnProperty(req.params.action)) {
        let action = actions[req.params.action];
        if (typeof action === 'function') {
            res.end(action(req.params.payload));
            return;
        }
    }
    res.end("Unsupported action.");
});

Running the query on it (using CodeQL 2.7.6) does not flag an alert.

Now I change the UnvalidatedDynamicMethCallQuery library by adding the following conjunct in its isSource predicate:

(...)  and
source.getStartLine() = 15

And suddenly I get an alert on the call to action.

Quite apart from the question of whether or not this alert is correct, I don't see how adding a conjunct to the isSource predicate, thereby making it smaller (in this particular case, one source instead of three), can lead to more alerts being reported.

[max-schaefer]

@asgerf suggested that this behaviour might be due to over-eager pruning of barrier guards: the call to action is only safe for the purposes of this query if it is both guarded by a hasOwnProperty check and a typeof ... === "function"' check. We prune barrier guards by a coarse forward scan, and by restricting our set of sources the former guard is pruned away.

This sounds plausible, since if I swap lines 14 and 15 I get consistent behaviour with both sets of sources. (I think it's the wrong behaviour, since it flags this as an alert, but at least it's consistent.) That would make this a soundness bug in the data flow analysis, which isn't too concerning (to me at least).