C++: Wire up models to DefaultTaintTracking

This adds support for arg-to-arg and arg-to-return taint.

Author: jbj

cpp/ql/src/semmle/code/cpp/ir/dataflow/DefaultTaintTracking.qll

@@ -4,6 +4,8 @@ private import semmle.code.cpp.ir.dataflow.DataFlow
 private import semmle.code.cpp.ir.dataflow.DataFlow2
 private import semmle.code.cpp.ir.IR
 private import semmle.code.cpp.ir.dataflow.internal.DataFlowDispatch as Dispatch
+private import semmle.code.cpp.models.interfaces.Taint
+private import semmle.code.cpp.models.interfaces.DataFlow
 
 /**
  * A predictable instruction is one where an external user can predict
@@ -159,18 +161,64 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   // This is part of the translation of `a[i]`, where we want taint to flow
   // from `a`.
   i2.(PointerAddInstruction).getLeft() = i1
-  // TODO: robust Chi handling
-  //
-  // TODO: Flow from argument to return of known functions: Port missing parts
-  // of `returnArgument` to the `interfaces.Taint` and `interfaces.DataFlow`
-  // libraries.
-  //
-  // TODO: Flow from input argument to output argument of known functions: Port
-  // missing parts of `copyValueBetweenArguments` to the `interfaces.Taint` and
-  // `interfaces.DataFlow` libraries and implement call side-effect nodes. This
-  // will help with the test for `ExecTainted.ql`. The test for
-  // `TaintedPath.ql` is more tricky because the output arg is a pointer
-  // addition expression.
+  or
+  // Flow from argument to return value
+  i2 = any(CallInstruction call |
+      exists(int indexIn |
+        modelTaintToReturnValue(call.getStaticCallTarget(), indexIn) and
+        i1 = call.getPositionalArgument(indexIn)
+      )
+    )
+  or
+  // Flow from input argument to output argument
+  // TODO: This won't work in practice as long as all aliased memory is tracked
+  // together in a single virtual variable.
+  // TODO: Will this work on the test for `TaintedPath.ql`, where the output arg
+  // is a pointer addition expression?
+  i2 = any(WriteSideEffectInstruction outNode |
+      exists(CallInstruction call, int indexIn, int indexOut |
+        modelTaintToParameter(call.getStaticCallTarget(), indexIn, indexOut) and
+        i1 = call.getPositionalArgument(indexIn) and
+        outNode.getIndex() = indexOut and
+        outNode.getPrimaryInstruction() = call
+      )
+    )
+}
+
+private predicate modelTaintToParameter(Function f, int parameterIn, int parameterOut) {
+  exists( FunctionInput modelIn, FunctionOutput modelOut |
+    f.(TaintFunction).hasTaintFlow(modelIn, modelOut) and
+    (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and
+    modelOut.isParameterDeref(parameterOut)
+  )
+}
+
+private predicate modelTaintToReturnValue(Function f, int parameterIn) {
+  // Taint flow from parameter to return value
+  exists(FunctionInput modelIn, FunctionOutput modelOut |
+    f.(TaintFunction).hasTaintFlow(modelIn, modelOut) and
+    (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and
+    (modelOut.isReturnValue() or modelOut.isReturnValueDeref())
+  )
+  or
+  // Data flow (not taint flow) to where the return value points. For the time
+  // being we will conflate pointers and objects in taint tracking.
+  exists(FunctionInput modelIn, FunctionOutput modelOut |
+    f.(DataFlowFunction).hasDataFlow(modelIn, modelOut) and
+    (modelIn.isParameter(parameterIn) or modelIn.isParameterDeref(parameterIn)) and
+    modelOut.isReturnValueDeref()
+  )
+  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(int parameterMid, InParameter modelMid, OutReturnValue returnOut |
+    modelTaintToParameter(f, parameterIn, parameterMid) and
+    modelMid.isParameter(parameterMid) and
+    f.(DataFlowFunction).hasDataFlow(modelMid, returnOut)
+  )
 }
 
 private Element adjustedSink(DataFlow::Node sink) {

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp

@@ -13,13 +13,13 @@ int main(int argc, char *argv[]) {
   int taintedInt = atoi(getenv("VAR"));
   taintedInt++; // BUG: `taintedInt` isn't marked as tainted. Only `++` is.
 
-  sink(_strdup(getenv("VAR"))); // BUG: no taint
+  sink(_strdup(getenv("VAR")));
   sink(strdup(getenv("VAR")));
   sink(unmodeled_function(getenv("VAR")));
 
   char untainted_buf[100] = "";
   char buf[100] = "VAR = ";
-  sink(strcat(buf, getenv("VAR"))); // BUG: no taint
+  sink(strcat(buf, getenv("VAR")));
 
   sink(buf); // BUG: no taint
   sink(untainted_buf); // the two buffers would be conflated if we added flow through partial chi inputs

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected

@@ -5,6 +5,9 @@
 | defaulttainttracking.cpp:13:25:13:30 | call to getenv | defaulttainttracking.cpp:13:25:13:37 | (const char *)... |
 | defaulttainttracking.cpp:13:25:13:30 | call to getenv | defaulttainttracking.cpp:14:3:14:14 | ... ++ |
 | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:6:15:6:24 | p#0 |
+| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
+| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:14 | call to _strdup |
+| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:29 | (const char *)... |
 | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:16:16:21 | call to getenv |
 | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:16:16:28 | (const char *)... |
 | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:5:14:5:23 | p#0 |
@@ -17,5 +20,8 @@
 | defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:18:27:18:32 | call to getenv |
 | defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:18:27:18:39 | (const char *)... |
 | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:3:38:3:39 | s2 |
+| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 |
+| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:13 | call to strcat |
+| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:33 | (const char *)... |
 | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:25 | call to getenv |
 | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:32 | (const char *)... |