Add PreGuardRecursivePredicate test

This commit is contained in:
Michael Hohn
2023-11-29 20:21:32 -08:00
committed by =Michael Hohn
parent 813a53a054
commit 788d772556
5 changed files with 228 additions and 0 deletions

View File

@@ -0,0 +1,133 @@
/**
* @kind path-problem
* @problem.severity warning
* @id javascript/example/multiflow
*/
import javascript
// XX: debug flow query
// import semmle.javascript.explore.ForwardDataFlow
import DataFlow::PathGraph
import DataFlow as DF
// Flow to consider:
//
// var value = this.getParameter('value'); //: source 1
// var ua = new GR('status'); //: source 2
// ua.setValue('status',value); //: taint step
// ua.update(); //: sink (if from source 2)
//
// var value = this.getParameter('value'); //: source 1
class ParameterSource extends CallExpr {
ParameterSource() {
exists(Expr inst |
this.getCalleeName() = "getParameter" and
(this.getReceiver().(ThisExpr) = inst or this.getReceiver().(Identifier) = inst)
)
}
}
// ua.setValue('status',value); //: taint step
predicate setValueTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
exists(DotExpr temp, MethodCallExpr mce, VarAccess gr, VarAccess postgr |
temp.getPropertyName() = "setValue" and
mce.getReceiver() = temp.getBase() and
gr = mce.getReceiver() and
pred.asExpr() = mce.getArgument(1) and
//
// Taint all accesses after setValue call.
// Trying data flow, this would be:
// succ = gr.flow().getASuccessor+() and
//
// Using control flow:
// 1. without sanitizer
// gr.getASuccessor+() = postgr and
// succ.asExpr() = postgr
//
// 2. with recursive predicate, no sanitizer
recursiveSuccessor(gr, postgr) and
succ.asExpr() = postgr
// // 3. with recursive predicate, with sanitizer
// sanitizerCheckedSuccessor(gr, postgr) and
// succ.asExpr() = postgr
)
}
predicate foo(VarAccess gr, VarAccess postgr) {
exists(DotExpr temp, MethodCallExpr mce |
temp.getPropertyName() = "setValue" and
mce.getReceiver() = temp.getBase() and
gr = mce.getReceiver() and
gr.getASuccessor+() = postgr
)
}
predicate foo1(Expr gr, Expr postgr) {
exists(DotExpr temp, MethodCallExpr mce |
temp.getPropertyName() = "setValue" and
mce.getReceiver() = temp.getBase() and
gr = mce.getReceiver() and
recursiveSuccessor(gr, postgr)
)
}
// Def-Use special handling:
// Include sanitizer check when flagging successive object member calls in taint step
predicate recursiveSuccessor(ControlFlowNode gr, ControlFlowNode postgr) {
gr.getASuccessor() = postgr
or
exists(ControlFlowNode p |
recursiveSuccessor(gr, p) and
p.getASuccessor() = postgr
)
// The final postgr needs to be a VarAccess for this query, but for the
// recursion we need to be able to traverse expressions.
}
// source 2 to sink flow
DF::SourceNode grType(DF::TypeTracker t) {
t.start() and
exists(GR gr | result.asExpr() = gr)
or
exists(DF::TypeTracker t2 | result = grType(t2).track(t2, t))
}
DF::SourceNode grType() { result = grType(DF::TypeTracker::end()) }
// ua.update(); //: sink (if from source 2)
DotExpr updateExpression() { result.getPropertyName() = "update" }
VarRef recordUpdate() { result = updateExpression().getBase() }
// var ua = new GR('status'); //: source 2
class GR extends NewExpr {
GR() { this.getCalleeName() = "GR" }
}
// The global flow configuration
class FromRequestToGrUpdate extends TaintTracking::Configuration {
FromRequestToGrUpdate() { this = "FromRequestToGrUpdate" }
override predicate isSource(DataFlow::Node source) {
exists(ParameterSource getParameter | source.asExpr() = getParameter)
}
override predicate isAdditionalTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
setValueTaintStep(pred, succ)
}
override predicate isSink(DataFlow::Node sink) {
exists(VarRef grUpdate |
sink.asExpr() = recordUpdate() and
grUpdate = sink.asExpr() and
grUpdate.getName() = "ua" and
// It's only a sink if it connects to source 2
grUpdate.flow().getALocalSource() = grType()
)
}
}
from FromRequestToGrUpdate dataflow, DataFlow::PathNode source, DataFlow::PathNode sink
where dataflow.hasFlowPath(source, sink)
select sink, source, sink, "Data flow from $@ to $@.", source, source.toString(), sink,
sink.toString()

View File

@@ -0,0 +1,48 @@
WARNING: Unused predicate foo (/Users/hohn/local/codeql-javascript-multiflow/solutions/PreGuardRecursivePredicate.ql:56,11-14)
WARNING: Unused predicate foo1 (/Users/hohn/local/codeql-javascript-multiflow/solutions/PreGuardRecursivePredicate.ql:65,11-15)
nodes
| sample-utility-0.js:5:13:5:46 | value |
| sample-utility-0.js:5:21:5:46 | this.ge ... value') |
| sample-utility-0.js:5:21:5:46 | this.ge ... value') |
| sample-utility-0.js:12:34:12:38 | value |
| sample-utility-0.js:17:34:17:38 | value |
| sample-utility-0.js:18:13:18:14 | ua |
| sample-utility-0.js:18:13:18:14 | ua |
| sample-utility-0.js:22:35:22:39 | value |
| sample-utility-0.js:23:13:23:14 | ua |
| sample-utility-0.js:23:13:23:14 | ua |
| sample-utility-1.js:2:9:2:42 | value |
| sample-utility-1.js:2:17:2:42 | this.ge ... value') |
| sample-utility-1.js:2:17:2:42 | this.ge ... value') |
| sample-utility-1.js:14:23:14:27 | value |
| sample-utility-1.js:15:2:15:3 | ua |
| sample-utility-1.js:15:2:15:3 | ua |
edges
| sample-utility-0.js:5:13:5:46 | value | sample-utility-0.js:12:34:12:38 | value |
| sample-utility-0.js:5:13:5:46 | value | sample-utility-0.js:17:34:17:38 | value |
| sample-utility-0.js:5:13:5:46 | value | sample-utility-0.js:22:35:22:39 | value |
| sample-utility-0.js:5:21:5:46 | this.ge ... value') | sample-utility-0.js:5:13:5:46 | value |
| sample-utility-0.js:5:21:5:46 | this.ge ... value') | sample-utility-0.js:5:13:5:46 | value |
| sample-utility-0.js:12:34:12:38 | value | sample-utility-0.js:12:34:12:38 | value |
| sample-utility-0.js:12:34:12:38 | value | sample-utility-0.js:22:35:22:39 | value |
| sample-utility-0.js:12:34:12:38 | value | sample-utility-0.js:23:13:23:14 | ua |
| sample-utility-0.js:12:34:12:38 | value | sample-utility-0.js:23:13:23:14 | ua |
| sample-utility-0.js:17:34:17:38 | value | sample-utility-0.js:17:34:17:38 | value |
| sample-utility-0.js:17:34:17:38 | value | sample-utility-0.js:18:13:18:14 | ua |
| sample-utility-0.js:17:34:17:38 | value | sample-utility-0.js:18:13:18:14 | ua |
| sample-utility-0.js:17:34:17:38 | value | sample-utility-0.js:22:35:22:39 | value |
| sample-utility-0.js:17:34:17:38 | value | sample-utility-0.js:23:13:23:14 | ua |
| sample-utility-0.js:17:34:17:38 | value | sample-utility-0.js:23:13:23:14 | ua |
| sample-utility-0.js:22:35:22:39 | value | sample-utility-0.js:22:35:22:39 | value |
| sample-utility-0.js:22:35:22:39 | value | sample-utility-0.js:23:13:23:14 | ua |
| sample-utility-0.js:22:35:22:39 | value | sample-utility-0.js:23:13:23:14 | ua |
| sample-utility-1.js:2:9:2:42 | value | sample-utility-1.js:14:23:14:27 | value |
| sample-utility-1.js:2:17:2:42 | this.ge ... value') | sample-utility-1.js:2:9:2:42 | value |
| sample-utility-1.js:2:17:2:42 | this.ge ... value') | sample-utility-1.js:2:9:2:42 | value |
| sample-utility-1.js:14:23:14:27 | value | sample-utility-1.js:14:23:14:27 | value |
| sample-utility-1.js:14:23:14:27 | value | sample-utility-1.js:15:2:15:3 | ua |
| sample-utility-1.js:14:23:14:27 | value | sample-utility-1.js:15:2:15:3 | ua |
#select
| sample-utility-0.js:18:13:18:14 | ua | sample-utility-0.js:5:21:5:46 | this.ge ... value') | sample-utility-0.js:18:13:18:14 | ua | Data flow from $@ to $@. | sample-utility-0.js:5:21:5:46 | this.ge ... value') | this.ge ... value') | sample-utility-0.js:18:13:18:14 | ua | ua |
| sample-utility-0.js:23:13:23:14 | ua | sample-utility-0.js:5:21:5:46 | this.ge ... value') | sample-utility-0.js:23:13:23:14 | ua | Data flow from $@ to $@. | sample-utility-0.js:5:21:5:46 | this.ge ... value') | this.ge ... value') | sample-utility-0.js:23:13:23:14 | ua | ua |
| sample-utility-1.js:15:2:15:3 | ua | sample-utility-1.js:2:17:2:42 | this.ge ... value') | sample-utility-1.js:15:2:15:3 | ua | Data flow from $@ to $@. | sample-utility-1.js:2:17:2:42 | this.ge ... value') | this.ge ... value') | sample-utility-1.js:15:2:15:3 | ua | ua |

View File

@@ -0,0 +1 @@
PreGuardRecursivePredicate.ql

View File

@@ -0,0 +1,29 @@
var SampleUtility = function(){};
SampleUtility.prototype = Object.extendsObject(Processor, {
setUserStatus: function() {
var value = this.getParameter('value');
var ua = new GR('users');
ua.query();
if(!ua.hasNext()){
ua.initialize();
ua.setValue('status',value);
ua.insert();
}
else {
ua.next();
ua.setValue('status',value); // unsafe
ua.update();
}
if (ua.safeToWrite()) {
ua.setValue('status', value); // safe
ua.update();
}
},
type: 'SampleUtility'
});

View File

@@ -0,0 +1,17 @@
var SampleUtility = function() {
var value = this.getParameter('value');
var ua = new GR('users');
ua.query();
if(!ua.hasNext()){
ua.initialize();
ua.setValue('status',value);
ua.insert();
}
else {
ua.next();
ua.setValue('status',value);
ua.update();
}
}