From e0c589060a30ccc040b1da01e33273d3d9c9e49c Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Mon, 18 Nov 2019 23:48:35 -0800 Subject: [PATCH 1/3] Split taintStep into many predicates --- ql/src/semmle/go/dataflow/TaintTracking.qll | 37 ++++++++++++++++----- 1 file changed, 28 insertions(+), 9 deletions(-) diff --git a/ql/src/semmle/go/dataflow/TaintTracking.qll b/ql/src/semmle/go/dataflow/TaintTracking.qll index c751d6d39da..32b9c7c7433 100644 --- a/ql/src/semmle/go/dataflow/TaintTracking.qll +++ b/ql/src/semmle/go/dataflow/TaintTracking.qll @@ -87,28 +87,35 @@ module TaintTracking { } } - /** - * Holds if taint flows from `pred` to `succ` in one step. - */ - private predicate taintStep(DataFlow::Node pred, DataFlow::Node succ) { + predicate referenceStep(DataFlow::Node pred, DataFlow::Node succ) { // if x is tainted, then so is &x succ.asExpr().(AddressExpr).getOperand() = pred.asExpr() or // if x is tainted, then so is *x succ.asExpr().(StarExpr).getBase() = pred.asExpr() - or + } + + predicate arrayStep(DataFlow::Node pred, DataFlow::Node succ) { // if an array is tainted, then so are all its elements succ.asExpr().(IndexExpr).getBase() = pred.asExpr() - or + } + + predicate tupleStep(DataFlow::Node pred, DataFlow::Node succ) { // if a tuple is tainted, then so are all its components succ = DataFlow::extractTupleElement(pred, _) - or + } + + predicate stringConcatStep(DataFlow::Node pred, DataFlow::Node succ) { // taint propagates through string concatenation succ.asExpr().(AddExpr).getAnOperand() = pred.asExpr() - or + } + + predicate sliceStep(DataFlow::Node pred, DataFlow::Node succ) { // taint propagates through slicing succ.asExpr().(SliceExpr).getBase() = pred.asExpr() - or + } + + predicate functionModelStep(DataFlow::Node pred, DataFlow::Node succ) { // step through function model exists(FunctionModel m, DataFlow::CallNode c, FunctionInput inp, FunctionOutput outp | c = m.getACall() and @@ -118,6 +125,18 @@ module TaintTracking { ) } + /** + * Holds if taint flows from `pred` to `succ` in one step. + */ + private predicate taintStep(DataFlow::Node pred, DataFlow::Node succ) { + referenceStep(pred, succ) or + arrayStep(pred, succ) or + tupleStep(pred, succ) or + stringConcatStep(pred, succ) or + sliceStep(pred, succ) or + functionModelStep(pred, succ) + } + /** * A model of a function specifying that the function propagates taint from * a parameter or qualifier to a result. From 09865a5f5cbfd553e5110ac6ee8d9077e6ecbd2e Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Mon, 18 Nov 2019 23:51:10 -0800 Subject: [PATCH 2/3] Add a field read taint step --- ql/src/semmle/go/dataflow/TaintTracking.qll | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ql/src/semmle/go/dataflow/TaintTracking.qll b/ql/src/semmle/go/dataflow/TaintTracking.qll index 32b9c7c7433..5033d155cb0 100644 --- a/ql/src/semmle/go/dataflow/TaintTracking.qll +++ b/ql/src/semmle/go/dataflow/TaintTracking.qll @@ -95,6 +95,11 @@ module TaintTracking { succ.asExpr().(StarExpr).getBase() = pred.asExpr() } + predicate fieldReadStep(DataFlow::Node pred, DataFlow::Node succ) { + // if x is tainted, then so is `x.y` + succ.(DataFlow::FieldReadNode).getBase() = pred + } + predicate arrayStep(DataFlow::Node pred, DataFlow::Node succ) { // if an array is tainted, then so are all its elements succ.asExpr().(IndexExpr).getBase() = pred.asExpr() @@ -130,6 +135,7 @@ module TaintTracking { */ private predicate taintStep(DataFlow::Node pred, DataFlow::Node succ) { referenceStep(pred, succ) or + fieldReadStep(pred, succ) or arrayStep(pred, succ) or tupleStep(pred, succ) or stringConcatStep(pred, succ) or From 3f437612e1ee31960e0f11bd43693dcd870fa6ba Mon Sep 17 00:00:00 2001 From: Sauyon Lee Date: Wed, 20 Nov 2019 11:27:10 -0800 Subject: [PATCH 3/3] Add qldoc to all taint step predicates. --- ql/src/semmle/go/dataflow/TaintTracking.qll | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/ql/src/semmle/go/dataflow/TaintTracking.qll b/ql/src/semmle/go/dataflow/TaintTracking.qll index 5033d155cb0..b23baaa7e66 100644 --- a/ql/src/semmle/go/dataflow/TaintTracking.qll +++ b/ql/src/semmle/go/dataflow/TaintTracking.qll @@ -87,41 +87,40 @@ module TaintTracking { } } + /** Holds if taint flows from `pred` to `succ` via a reference or dereference. */ predicate referenceStep(DataFlow::Node pred, DataFlow::Node succ) { - // if x is tainted, then so is &x succ.asExpr().(AddressExpr).getOperand() = pred.asExpr() or - // if x is tainted, then so is *x succ.asExpr().(StarExpr).getBase() = pred.asExpr() } + /** Holds if taint flows from `pred` to `succ` via a field read. */ predicate fieldReadStep(DataFlow::Node pred, DataFlow::Node succ) { - // if x is tainted, then so is `x.y` succ.(DataFlow::FieldReadNode).getBase() = pred } + /** Holds if taint flows from `pred` to `succ` via an array index operation. */ predicate arrayStep(DataFlow::Node pred, DataFlow::Node succ) { - // if an array is tainted, then so are all its elements succ.asExpr().(IndexExpr).getBase() = pred.asExpr() } + /** Holds if taint flows from `pred` to `succ` via an extract tuple operation. */ predicate tupleStep(DataFlow::Node pred, DataFlow::Node succ) { - // if a tuple is tainted, then so are all its components succ = DataFlow::extractTupleElement(pred, _) } + /** Holds if taint flows from `pred` to `succ` via string concatenation. */ predicate stringConcatStep(DataFlow::Node pred, DataFlow::Node succ) { - // taint propagates through string concatenation succ.asExpr().(AddExpr).getAnOperand() = pred.asExpr() } + /** Holds if taint flows from `pred` to `succ` via a slice operation. */ predicate sliceStep(DataFlow::Node pred, DataFlow::Node succ) { - // taint propagates through slicing succ.asExpr().(SliceExpr).getBase() = pred.asExpr() } + /** Holds if taint flows from `pred` to `succ` via a function model. */ predicate functionModelStep(DataFlow::Node pred, DataFlow::Node succ) { - // step through function model exists(FunctionModel m, DataFlow::CallNode c, FunctionInput inp, FunctionOutput outp | c = m.getACall() and m.hasTaintFlow(inp, outp) and