github / github/codeql

JavaScript: Restricting `isSource` predicate leads to more alerts

Open
#7,790 1 comment 0 reactions 0 assignees View on GitHub
bug JS
Dominant language
CodeQL
Stars
10.1k
Forks
2.1k
Avg merge
2d 15h
Merged PRs (30d)
141

Description

I have the following test file for the `UnvalidatedDynamicMethodCall` query:

```js
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:

```ql
(...) 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.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.