github / github/codeql

[JS] Can't use specific path nodes of Configuration1 in Configuration2 definition (throws non-monotonic recursion error)

Abierto
#8,341 0 comentarios 0 reacciones 0 asignados Ver en GitHub
question
Lenguaje dominante
CodeQL
Estrellas
10.1k
Forks
2.1k
Merge medio
2 d 15 h
PR fusionados (30 d)
141

Descripción

My goal is to capture the declaration/initialization of the dynamic object for [unvalidated dynamic method call vulnerability](https://codeql.github.com/codeql-query-help/javascript/js-unvalidated-dynamic-method-call/).
So for instance, in the following code, corresponding to dynamic variable indexing in lines 11 and 14, I want to detect `actions = {...` and `actions2 = new Map...` as dynamic objection initializations in lines 4 and 6.
```js
var express = require('express');
var app = express();

var actions = {/*..*/}; // <--- line 4
actions['a'] = function(){}
var actions2 = new Map(); // <--- line 6
actions2.set('a', function(){})

function run(){
app.get('/perform/:action/:payload', function(req, res) {
let action = actions[req.params.action]; // <--- line 11
res.end(action(req.params.payload));

let action2 = actions[req.params.action]; // <--- line 14
res.end(action2(req.params.payload));

});
}
```
Is there some simple solution to do this? Below I will describe my attempt to solve this problem and where I am currently stuck at.

### My Approach
Initially, I tried to do this using the local dataflow (`getASuccessor*()`) and returned the results via a query predicate.
However, local dataflow is not complete and I realized I would need to use global dataflow. The source would be any `sourceNode` and sink would be particular nodes on the dataflow graph of the original configuration. Therefore, I created the following `Configuration`

```codeql
class ContextConfiguration extends DataFlow::Configuration{
ContextConfiguration() {this = "ContextConfiguration"}
override predicate isSource(DataFlow::Node source) { exists(DataFlow::SourceNode src | src = source) }

override predicate isSink(DataFlow::Node sink) {
/* CASE 1 - if some node on path is getting read as property
*/
exists(OriginalConfiguration cfg,
DataFlow::PropRead read, DataFlow::Node propNameNode |
(
DataFlow::onPath(propNameNode, cfg, _) // <-- this predicate was made public in Configuration.qll and causes Error
)
and
propNameNode = read.getPropertyNameExpr().flow() and // this case just selects `actions` in `actions[req.params.action]`
sink.getASuccessor() = read.getBase() // `actions` is `sink`, `req.params.action` is `propNameNode`
) or
/* CASE 2 - if some node on path is getting into argument of get function
*/
exists(OriginalConfiguration cfg,
DataFlow::SourceNode base, DataFlow::CallNode get, DataFlow::Node propNameNode |
(
DataFlow::onPath(propNameNode, cfg, _)
)
and
get = base.getAMethodCall("get") and // this case just selects `actions2` in `actions2.get()`
propNameNode = get.getArgument(0) and // `actions2` is `sink`, `req.params.action` is `propNameNode`
sink = get.getReceiver()
)
}
}
```

But this throws the following error
```codeql
Non-monotonic recursion: FindSanitizers::ContextConfiguration::isSink --> Configuration::onPath -->
Configuration::reachableFromSource -!-> TaintTracking::TaintTracking::Configuration::isLabeledBarrier -->
Configuration::Configuration::isLabeledBarrier --> Configuration::barrierGuardBlocksNode -->
Configuration::barrierGuardBlocksAccessPath --> Configuration::barrierGuardIsRelevant --> Configuration::isRelevantForward -->
Configuration::isLive --> Configuration::isSink --> FindSanitizers::ContextConfiguration::isSink
```
I understand that `onPath` function usually depends on the `isSink` predicate. However, I call `onPath` with a specific `Configuration` as an argument and I do not see how it could lead to any recursion. Can someone explain if this is expected and if it can be resolved somehow?

Thanks!

----
**EDIT** : I was going through docs and is `TypeTracking` intended for this purpose?

Guía de contribución

Abrir la guía de contribución

Evaluación

Este issue todavía no se ha evaluado.

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.