github / github/codeql

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

Đang mở
#8,341 0 bình luận 0 reaction 0 người được giao Xem trên GitHub
question
Ngôn ngữ chính
CodeQL
Star
10.1k
Fork
2.1k
Merge trung bình
2 ngày 15 giờ
Pull request đã merge (30 ngày)
141

Mô tả

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?

Hướng dẫn đóng góp

Mở hướng dẫn đóng góp

Đánh giá

Issue này chưa được đánh giá.

Nhận issue mới trong hộp thư của bạn

Bản tóm tắt ngắn những issue GitHub phù hợp với người mới.