-
Notifications
You must be signed in to change notification settings - Fork 1.9k
isSanitizerGuard is not works, what should i do? #15543
-
I'm a fishman who learning about codeql to find vulnerabilities for nodejs express
I was writing a codeql snippets for finding all about command injection in nodejs express. Then i found a false positive. Here is my codeql snippet
/**
* @name rce-from-user-input
* @kind path-problem
* @problem.severity warning
* @tags correctness
* @id js
*/
import javascript
import semmle.javascript.frameworks.Express
import DataFlow::PathGraph
class MyDataFlowConfiguration extends TaintTracking::Configuration {
MyDataFlowConfiguration() { this = "MyDataFlowConfiguration" }
override predicate isSource(DataFlow::Node source) {
Express::isRequest(source)
}
override predicate isSink(DataFlow::Node sink) {
exists(DataFlow::CallNode call |
call = DataFlow::moduleMember("child_process", "exec").getACall() and
call.getArgument(0) = sink
)
}
override predicate isSanitizerGuard(TaintTracking::SanitizerGuardNode nd) {
nd instanceof CheckPathSanitizerGuard
}
}
class CheckPathSanitizerGuard extends TaintTracking::SanitizerGuardNode, DataFlow::CallNode {
CheckPathSanitizerGuard() { this.getCalleeName() = "validateFileName" }
override predicate sanitizes(boolean outcome, Expr e) {
outcome = false and
e = getArgument(0).asExpr()
}
}
from DataFlow::PathNode source, DataFlow::PathNode sink, MyDataFlowConfiguration f
where f.hasFlowPath(source, sink)
select sink.getNode(), source, sink, "$@ flows to $@ ", source.getNode(), source.toString(), sink.getNode(), sink.toString()
My nodejs snippet like below
var report_name = req.query.report_name;
if (process.platform.indexOf('linux') > -1) {
var downPath = '/hdisk/exports_report/reportFile' + id;
var zipfilename = report_name + '.tgz';
const exists = fs.existsSync(downPath + '/report_html.html');
if (!validateFileName(zipfilename)){
console.log('tgz error: invaild filename' + zipfilename);
res.render('error/error');
}
var cmdAddZip = 'cd ' + downPath + ' \r\n tar -zcvf ' + zipfilename + ' ' + (exists ? 'report_html.html' : '') + ' v2';
exec(cmdAddZip)
It has a flow path from express of req to exec. But req.query has been sanitized by validateFileName function.
But whatever i rewritten predicate isSanitizerGuard, the sanitizer didn't works.I couldn't find any documents about that?
Beta Was this translation helpful? Give feedback.
All reactions
It looks like exec(cmdAddZip) is being executed even if !validateFileName(zipfilename) is true. I think you need to add an else block to the if statement where you cll exec(cmdAddZip).
Replies: 1 comment 1 reply
-
It looks like exec(cmdAddZip) is being executed even if !validateFileName(zipfilename) is true. I think you need to add an else block to the if statement where you cll exec(cmdAddZip).
Beta Was this translation helpful? Give feedback.
All reactions
-
thanks , after add return statement in if statement scope , the SanitizerGuard has been workd.
Beta Was this translation helpful? Give feedback.
All reactions
-
👍 2