github / github/codeql

isSanitizerGuard works incorrectly when the function name startwith "isValid"

オープン
#17,393 コメント 1 件 リアクション 0 件 担当者 0 名 GitHub で見る
JS question
主要言語
CodeQL
スター
10.1k
フォーク
2.1k
平均マージ
2日 15時間
マージ済み PR(30日)
141

説明

i am try the the codeql query from https://codeql.github.com/docs/codeql-language-guides/analyzing-data-flow-in-javascript-and-typescript/ ;
the code to analysis is copied from tutorial
```
const fs = require('fs'),
path = require('path');

function readFileHelper(p) { // #2
if (!checkPath(p)){
return;
}
fs.readFile(p, // #4
'utf8', (err, data) => {
if (err) throw err;
console.log(data);
});
}

readFileHelper(process.argv[2]); // #1
```
and the query is also from tutorial
```
/**
* @name Find usage of request.body and request.query in Express.js
* @description Finds variables assigned from request.body and request.query in Express.js applications.
* @kind problem
* @id js/express-request-body-query
*/
import javascript

class CheckPathSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {
CheckPathSanitizerGuard() { this.getCalleeName() = "checkPath" }

override predicate sanitizes(boolean outcome, Expr e) {
outcome = true and
e = getArgument(0).asExpr()
}
}

class CommandLineFileNameConfiguration extends TaintTracking::Configuration {
CommandLineFileNameConfiguration() { this = "CommandLineFileNameConfiguration" }

override predicate isSource(DataFlow::Node source) {
DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyRead() = source
}

override predicate isSink(DataFlow::Node sink) {
DataFlow::moduleMember("fs", "readFile").getACall().getArgument(0) = sink
}
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode nd) {
nd instanceof CheckPathSanitizerGuard
}
}

from CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink
where cfg.hasFlow(source, sink)
select source, "12312323"
```
i use command line cli (2.18.3 newest version) to get the result
```
codeql database create codeqldb --language=javascript
codeql database analyze --rerun --threads 0 /test2.ql --format=sarif-latest --output=1.sarif
```
It all works fine.
Then i change the ql, this line
```
override predicate sanitizes(boolean outcome, Expr e) {
outcome = true and
e = getArgument(0).asExpr()
}
```
to
```
override predicate sanitizes(boolean outcome, Expr e) {
outcome = false and
e = getArgument(0).asExpr()
}
```
As expected, it does not sanitize the output.
The wierd thing is, if i change the function name of "checkPath", to "isValid", the sanitizer works very wierd, it sanitize the output no matter outcome is true of false. code is like:
```
const fs = require('fs'),
path = require('path');

function readFileHelper(p) { // #2
if (!isValid(p)){
return;
}
fs.readFile(p, // #4
'utf8', (err, data) => {
if (err) throw err;
console.log(data);
});
}

readFileHelper(process.argv[2]); // #1
```
ql is like
```
/**
* @name Find usage of request.body and request.query in Express.js
* @description Finds variables assigned from request.body and request.query in Express.js applications.
* @kind problem
* @id js/express-request-body-query
*/
import javascript

class CheckPathSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {
CheckPathSanitizerGuard() { this.getCalleeName() = "isValid" }

override predicate sanitizes(boolean outcome, Expr e) {
outcome = false and
e = getArgument(0).asExpr()
}
}

class CommandLineFileNameConfiguration extends TaintTracking::Configuration {
CommandLineFileNameConfiguration() { this = "CommandLineFileNameConfiguration" }

override predicate isSource(DataFlow::Node source) {
DataFlow::globalVarRef("process").getAPropertyRead("argv").getAPropertyRead() = source
}

override predicate isSink(DataFlow::Node sink) {
DataFlow::moduleMember("fs", "readFile").getACall().getArgument(0) = sink
}
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode nd) {
nd instanceof CheckPathSanitizerGuard
}
}

from CommandLineFileNameConfiguration cfg, DataFlow::Node source, DataFlow::Node sink
where cfg.hasFlow(source, sink)
select source, "12312323"
```

The only thing changed is the function name. "isValid" is not some special thing in js i think. I try other function names, everything start with isValid works abnormal, like "isValidd" "isValidg", anything else works correctly, like "isVali"

コントリビューションガイド

コントリビューションガイドを開く

評価

この issue はまだ評価されていません。

新しい issue をメールで受け取る

初心者向けの GitHub issue を短くまとめたダイジェスト。