C++: Some data flow through partial chi operands

Author: jbj

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

@@ -278,12 +278,18 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
   // By allowing flow through the total operand, we ensure that flow is not lost
   // due to shortcomings of the alias analysis. We may get false flow in cases
   // where the data is indeed overwritten.
-  //
-  // Allowing flow through the partial operand would be more noisy, especially
-  // for variables that have escaped: for soundness, the IR has to assume that
-  // every write to an unknown address can affect every escaped variable, and
-  // this assumption shows up as data flowing through partial chi operands.
   iTo.getAnOperand().(ChiTotalOperand).getDef() = iFrom
+  or
+  // Flow through the partial operand must be restricted a bit more. For
+  // soundness, the IR has to assume that every write to an unknown address can
+  // affect every escaped variable, and this assumption shows up as data flowing
+  // through partial chi operands. The chi instructions for all escaped data can
+  // be recognized by having unknown types. For all other chi instructions, flow
+  // through partial operands is more likely to be real.
+  exists(ChiInstruction chi | iTo = chi |
+    iFrom = chi.getPartial() and
+    not chi.getResultIRType() instanceof IRUnknownType
+  )
 }
 
 /**

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

@@ -21,8 +21,8 @@ int main(int argc, char *argv[]) {
   char buf[100] = "VAR = ";
   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
+  sink(buf);
+  sink(untainted_buf); // the two buffers would be conflated if we added flow through all partial chi inputs
 
   return 0;
 }

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

@@ -25,6 +25,7 @@
 | 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:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | buf |
 | 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 |

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

@@ -107,9 +107,9 @@ void array_test(int i) {
 	arr3[5] = 0;
 
 	sink(arr1[5]); // tainted
-	sink(arr1[i]); // tainted [NOT DETECTED]
-	sink(arr2[5]); // tainted [NOT DETECTED]
-	sink(arr2[i]); // tainted [NOT DETECTED]
+	sink(arr1[i]); // tainted [NOT DETECTED with AST]
+	sink(arr2[5]); // tainted [NOT DETECTED with AST]
+	sink(arr2[i]); // tainted [NOT DETECTED with AST]
 	sink(arr3[5]);
 	sink(arr3[i]);
 }

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

@@ -7,6 +7,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

@@ -2,6 +2,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 |