C++: Suppress FieldAddressInstruction taint

See code comment. This fixes false positives on openjdk/jdk.

Author: jbj

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

@@ -187,7 +187,15 @@ private predicate instructionTaintStep(Instruction i1, Instruction i2) {
   // Flow through pointer dereference
   i2.(LoadInstruction).getSourceAddress() = i1
   or
-  i2.(UnaryInstruction).getUnary() = i1
+  // Unary instructions tend to preserve enough information in practice that we
+  // want taint to flow through.
+  // The exception is `FieldAddressInstruction`. Together with the rule for
+  // `LoadInstruction` above and for `ChiInstruction` below, flow through
+  // `FieldAddressInstruction` could cause flow into one field to come out an
+  // unrelated field. This would happen across function boundaries, where the IR
+  // would not be able to match loads to stores.
+  i2.(UnaryInstruction).getUnary() = i1 and
+  not i2 instanceof FieldAddressInstruction
   or
   i2.(ChiInstruction).getPartial() = i1 and
   not i2.isResultConflated()

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

@@ -93,8 +93,8 @@ struct Point {
   int y;
 
   void callSink() {
-    sink(this->x); // tainted
-    sink(this->y); // not tainted [FALSE POSITIVE]
+    sink(this->x); // tainted [NOT DETECTED]
+    sink(this->y); // not tainted
   }
 };
 

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

@@ -103,9 +103,7 @@
 | defaulttainttracking.cpp:88:18:88:23 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:10:11:10:13 | p#0 |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:96:10:96:13 | this |
-| defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:96:16:96:16 | x |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:97:10:97:13 | this |
-| defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:97:16:97:16 | y |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:103:9:103:14 | call to getenv |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:103:9:103:24 | (int)... |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:103:9:103:24 | access to array |

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

@@ -17,9 +17,7 @@
 | defaulttainttracking.cpp:88:18:88:23 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | IR only |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:10:11:10:13 | p#0 | IR only |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:96:10:96:13 | this | IR only |
-| defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:96:16:96:16 | x | IR only |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:97:10:97:13 | this | IR only |
-| defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:97:16:97:16 | y | IR only |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:103:5:103:5 | x | AST only |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:104:10:104:10 | x | IR only |
 | defaulttainttracking.cpp:103:9:103:14 | call to getenv | defaulttainttracking.cpp:106:3:106:3 | p | IR only |