C++: Flow through read side effects

Until we have better tracking of indirections, these flow rules conflate
pointers and their contents.

Author: jbj

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

@@ -166,7 +166,7 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   i2 = any(CallInstruction call |
       exists(int indexIn |
         modelTaintToReturnValue(call.getStaticCallTarget(), indexIn) and
-        i1 = call.getPositionalArgument(indexIn)
+        i1 = getACallArgumentOrIndirection(call, indexIn)
       )
     )
   or
@@ -178,13 +178,28 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   i2 = any(WriteSideEffectInstruction outNode |
       exists(CallInstruction call, int indexIn, int indexOut |
         modelTaintToParameter(call.getStaticCallTarget(), indexIn, indexOut) and
-        i1 = call.getPositionalArgument(indexIn) and
+        i1 = getACallArgumentOrIndirection(call, indexIn) and
         outNode.getIndex() = indexOut and
         outNode.getPrimaryInstruction() = call
       )
     )
 }
 
+/**
+ * Get an instruction that goes into argument `argumentIndex` of `call`. This
+ * can be either directly or through one pointer indirection.
+ */
+private Instruction getACallArgumentOrIndirection(CallInstruction call, int argumentIndex) {
+  result = call.getPositionalArgument(argumentIndex)
+  or
+  exists(ReadSideEffectInstruction readSE |
+    // TODO: why are read side effect operands imprecise?
+    result = readSE.getSideEffectOperand().getAnyDef() and
+    readSE.getPrimaryInstruction() = call and
+    readSE.getIndex() = argumentIndex
+  )
+}
+
 private predicate modelTaintToParameter(Function f, int parameterIn, int parameterOut) {
   exists(FunctionInput modelIn, FunctionOutput modelOut |
     f.(TaintFunction).hasTaintFlow(modelIn, modelOut) and

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

@@ -25,7 +25,11 @@
 | 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 *)... |
+| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:32:11:32:26 | p#0 |
 | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:11:38:21 | env_pointer |
 | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:25:38:30 | call to getenv |
 | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:25:38:37 | (void *)... |
+| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:22:39:22 | a |
+| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:26:39:34 | call to inet_addr |
 | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:50:39:61 | & ... |
+| defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:40:10:40:10 | a |