From 8cdb8ef0dd0c20fd27bca1ac414f020d601fdf23 Mon Sep 17 00:00:00 2001 From: Michael Hohn Date: Tue, 28 Nov 2023 16:39:13 -0800 Subject: [PATCH] Add new example to illustrate taint propagation with def-use dataflow --- README.org | 2 +- js-sqli.code-workspace | 4 -- sample-utility-0.js | 23 ++++++++++++ sample-utility-1.js | 17 +++++++++ session/session1.ql | 85 ++++++++++++++++++++++++++++++++++++++++++ 5 files changed, 126 insertions(+), 5 deletions(-) create mode 100644 sample-utility-0.js create mode 100644 sample-utility-1.js create mode 100644 session/session1.ql diff --git a/README.org b/README.org index 0803dd4..bebe958 100644 --- a/README.org +++ b/README.org @@ -239,7 +239,7 @@ codeql database create --language=javascript -s . -j 8 -v $DB # Check it - unzip -v js-sqli-db-*/src.zip |grep add + unzip -v $DB/src.zip |egrep '(add|sample)' #+END_SRC 12. add the database to the editor. To do this there is a widget on the left diff --git a/js-sqli.code-workspace b/js-sqli.code-workspace index ed39235..162d985 100644 --- a/js-sqli.code-workspace +++ b/js-sqli.code-workspace @@ -2,10 +2,6 @@ "folders": [ { "path": "." - }, - { - "name": "[js-sqli-db-c860686 source archive]", - "uri": "codeql-zip-archive://0-72/Users/hohn/local/codeql-javascript-multiflow/js-sqli-db-c860686/src.zip" } ], "settings": { diff --git a/sample-utility-0.js b/sample-utility-0.js new file mode 100644 index 0000000..f26d073 --- /dev/null +++ b/sample-utility-0.js @@ -0,0 +1,23 @@ +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); + ua.update(); + } + }, + + type: 'SampleUtility' +}); diff --git a/sample-utility-1.js b/sample-utility-1.js new file mode 100644 index 0000000..aea3135 --- /dev/null +++ b/sample-utility-1.js @@ -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(); + } +} diff --git a/session/session1.ql b/session/session1.ql new file mode 100644 index 0000000..072f257 --- /dev/null +++ b/session/session1.ql @@ -0,0 +1,85 @@ +/** + * @kind path-problem + * @problem.severity warning + * @id javascript/example/multiflow + */ + +import javascript +// XX: debug flow query +// import semmle.javascript.explore.ForwardDataFlow +import DataFlow::PathGraph + +// 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: + gr.getASuccessor+() = postgr and + succ.asExpr() = postgr + ) + +} + +VarRef methodCalled(string name) { + exists(DotExpr temp | + temp.getPropertyName() = name and + result = temp.getBase() + ) +} + +// 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" } +} + +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" + ) + } +} + +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()