Merge pull request github#3320 from Semmle/rdmarsh/cpp/taint-tracking-util-port

C++: move logic from DefaultTaintTracking into TaintTrackingUtil

Author: jbj

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/ModelUtil.qll

@@ -0,0 +1,47 @@
+/**
+ * Provides predicates for mapping the `FunctionInput` and `FunctionOutput`
+ * classes used in function models to the corresponding instructions.
+ */
+
+private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.dataflow.DataFlow
+
+/**
+ * Gets the instruction that goes into `input` for `call`.
+ */
+Instruction callInput(CallInstruction call, FunctionInput input) {
+  // A positional argument
+  exists(int index |
+    result = call.getPositionalArgument(index) and
+    input.isParameter(index)
+  )
+  or
+  // A value pointed to by a positional argument
+  exists(ReadSideEffectInstruction read |
+    result = read and
+    read.getPrimaryInstruction() = call and
+    input.isParameterDeref(read.getIndex())
+  )
+  or
+  // The qualifier pointer
+  result = call.getThisArgument() and
+  input.isQualifierAddress()
+  //TODO: qualifier deref
+}
+
+/**
+ * Gets the instruction that holds the `output` for `call`.
+ */
+Instruction callOutput(CallInstruction call, FunctionOutput output) {
+  // The return value
+  result = call and
+  output.isReturnValue()
+  or
+  // The side effect of a call on the value pointed to by a positional argument
+  exists(WriteSideEffectInstruction effect |
+    result = effect and
+    effect.getPrimaryInstruction() = call and
+    output.isParameterDeref(effect.getIndex())
+  )
+  // TODO: qualifiers, return value dereference
+}

cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/TaintTrackingUtil.qll

@@ -1,5 +1,8 @@
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.ir.dataflow.DataFlow
+private import ModelUtil
+private import semmle.code.cpp.models.interfaces.DataFlow
+private import semmle.code.cpp.models.interfaces.SideEffect
 
 /**
  * Holds if taint propagates from `nodeFrom` to `nodeTo` in exactly one local
@@ -45,6 +48,25 @@ private predicate localInstructionTaintStep(Instruction nodeFrom, Instruction no
   )
   or
   nodeTo.(LoadInstruction).getSourceAddress() = nodeFrom
+  or
+  modeledInstructionTaintStep(nodeFrom, nodeTo)
+  or
+  // Flow through partial reads of arrays and unions
+  nodeTo.(LoadInstruction).getSourceValueOperand().getAnyDef() = nodeFrom and
+  not nodeFrom.isResultConflated() and
+  (
+    nodeFrom.getResultType() instanceof ArrayType or
+    nodeFrom.getResultType() instanceof Union
+  )
+  or
+  // Flow from an element to an array or union that contains it.
+  nodeTo.(ChiInstruction).getPartial() = nodeFrom and
+  not nodeTo.isResultConflated() and
+  exists(Type t | nodeTo.getResultLanguageType().hasType(t, false) |
+    t instanceof Union
+    or
+    t instanceof ArrayType
+  )
 }
 
 /**
@@ -82,3 +104,34 @@ predicate defaultAdditionalTaintStep(DataFlow::Node src, DataFlow::Node sink) {
  * but not in local taint.
  */
 predicate defaultTaintBarrier(DataFlow::Node node) { none() }
+
+/**
+ * Holds if taint can flow from `instrIn` to `instrOut` through a call to a
+ * modeled function.
+ */
+predicate modeledInstructionTaintStep(Instruction instrIn, Instruction instrOut) {
+  exists(CallInstruction call, TaintFunction func, FunctionInput modelIn, FunctionOutput modelOut |
+    instrIn = callInput(call, modelIn) and
+    instrOut = callOutput(call, modelOut) and
+    call.getStaticCallTarget() = func and
+    func.hasTaintFlow(modelIn, modelOut)
+  )
+  or
+  // Taint flow from one argument to another and data flow from an argument to a
+  // return value. This happens in functions like `strcat` and `memcpy`. We
+  // could model this flow in two separate steps, but that would add reverse
+  // flow from the write side-effect to the call instruction, which may not be
+  // desirable.
+  exists(
+    CallInstruction call, Function func, FunctionInput modelIn, OutParameterDeref modelMidOut,
+    int indexMid, InParameter modelMidIn, OutReturnValue modelOut
+  |
+    instrIn = callInput(call, modelIn) and
+    instrOut = callOutput(call, modelOut) and
+    call.getStaticCallTarget() = func and
+    func.(TaintFunction).hasTaintFlow(modelIn, modelMidOut) and
+    func.(DataFlowFunction).hasDataFlow(modelMidIn, modelOut) and
+    modelMidOut.isParameterDeref(indexMid) and
+    modelMidIn.isParameter(indexMid)
+  )
+}

cpp/ql/test/library-tests/dataflow/taint-tests/test_diff.expected

@@ -22,6 +22,9 @@
 | taint.cpp:93:11:93:11 | taint.cpp:71:22:71:27 | AST only |
 | taint.cpp:94:11:94:11 | taint.cpp:72:7:72:12 | AST only |
 | taint.cpp:109:7:109:13 | taint.cpp:105:12:105:17 | IR only |
+| taint.cpp:110:7:110:13 | taint.cpp:105:12:105:17 | IR only |
+| taint.cpp:111:7:111:13 | taint.cpp:106:12:106:17 | IR only |
+| taint.cpp:112:7:112:13 | taint.cpp:106:12:106:17 | IR only |
 | taint.cpp:130:7:130:9 | taint.cpp:127:8:127:13 | IR only |
 | taint.cpp:137:7:137:9 | taint.cpp:120:11:120:16 | AST only |
 | taint.cpp:173:8:173:13 | taint.cpp:164:19:164:24 | AST only |

cpp/ql/test/library-tests/dataflow/taint-tests/test_ir.expected

@@ -4,6 +4,9 @@
 | taint.cpp:16:8:16:14 | source1 | taint.cpp:12:22:12:27 | call to source |
 | taint.cpp:17:8:17:16 | ++ ... | taint.cpp:12:22:12:27 | call to source |
 | taint.cpp:109:7:109:13 | access to array | taint.cpp:105:12:105:17 | call to source |
+| taint.cpp:110:7:110:13 | access to array | taint.cpp:105:12:105:17 | call to source |
+| taint.cpp:111:7:111:13 | access to array | taint.cpp:106:12:106:17 | call to source |
+| taint.cpp:112:7:112:13 | access to array | taint.cpp:106:12:106:17 | call to source |
 | taint.cpp:129:7:129:9 | * ... | taint.cpp:120:11:120:16 | call to source |
 | taint.cpp:130:7:130:9 | * ... | taint.cpp:127:8:127:13 | call to source |
 | taint.cpp:134:7:134:9 | * ... | taint.cpp:120:11:120:16 | call to source |