C++: Respond to review comments and accept test changes.

Author: MathiasVP

cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/annotate_path_to_sink/dispatch.cpp

@@ -28,10 +28,10 @@ void testStruct() {
   globalStruct.sinkPtr(atoi(getenv("TAINTED"))); // $ ir MISSING: ast
   globalStruct.notSinkPtr(atoi(getenv("TAINTED"))); // clean
 
-  globalUnion.sinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path=31:28 ir-path=32:31 ir-path=34:22 
-  globalUnion.notSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path=31:28 ir-path=32:31 ir-path=34:22
+  globalUnion.sinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path
+  globalUnion.notSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path
 
-  globalSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path=31:28 ir-path=32:31 ir-path=34:22
+  globalSinkPtr(atoi(getenv("TAINTED"))); // $ ast ir-path
 }
 
 class B {
@@ -55,12 +55,12 @@ class D3 : public D2 {
 
 void test_dynamic_cast() {
     B* b = new D3();
-    b->f(getenv("VAR")); // $ ast ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
+    b->f(getenv("VAR")); // $ ast ir-path
 
-    ((D2*)b)->f(getenv("VAR")); // $ ast ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
-    static_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
-    dynamic_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
-    reinterpret_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
+    ((D2*)b)->f(getenv("VAR")); // $ ast ir-path
+    static_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path
+    dynamic_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path
+    reinterpret_cast<D2*>(b)->f(getenv("VAR")); // $ ast ir-path
 
     B* b2 = new D2();
     b2->f(getenv("VAR"));
@@ -70,5 +70,5 @@ void test_dynamic_cast() {
     dynamic_cast<D2*>(b2)->f(getenv("VAR"));
     reinterpret_cast<D2*>(b2)->f(getenv("VAR"));
 
-    dynamic_cast<D3*>(b2)->f(getenv("VAR")); // $ SPURIOUS: ast ir-path=58:10 ir-path=60:17 ir-path=61:28 ir-path=62:29 ir-path=63:33 ir-path=73:30
+    dynamic_cast<D3*>(b2)->f(getenv("VAR")); // $ SPURIOUS: ast ir-path
 }

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

@@ -7,6 +7,7 @@ import cpp
 import semmle.code.cpp.security.TaintTrackingImpl as ASTTaintTracking
 import semmle.code.cpp.ir.dataflow.DefaultTaintTracking as IRDefaultTaintTracking
 import IRDefaultTaintTracking::TaintedWithPath as TaintedWithPath
+import TaintedWithPath::Private
 import TestUtilities.InlineExpectationsTest
 
 predicate isSinkArgument(Element sink) {
@@ -25,8 +26,15 @@ class SourceConfiguration extends TaintedWithPath::TaintTrackingConfiguration {
 predicate irTaint(Element source, Element sink, string tag) {
   exists(TaintedWithPath::PathNode sinkNode, TaintedWithPath::PathNode predNode |
     TaintedWithPath::taintedWithPath(source, _, _, sinkNode) and
-    predNode = TaintedWithPath::Private::getAPredecessor*(sinkNode) and
-    sink = TaintedWithPath::Private::getElementFromPathNode(predNode) and
+    predNode = getAPredecessor*(sinkNode) and
+    sink = getElementFromPathNode(predNode) and
+    // Make sure the path is actually reachable from this predecessor.
+    // Otherwise, we could pick `predNode` to be b when `source` is
+    // `source1` in this dataflow graph:
+    // source1 ---> a ---> c ---> sinkNode
+    //                   ^
+    // source2 ---> b --/
+    source = getElementFromPathNode(getAPredecessor*(predNode)) and
     if sinkNode = predNode then tag = "ir-sink" else tag = "ir-path"
   )
 }

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

@@ -79,7 +79,7 @@ class Derived2 : public Derived1 {
 class Derived3 : public Derived2 {
     public:
     void f(const char* p) override { // $ ir-path=124:19 ir-path=126:43 ir-path=128:44
-        sink(p); // $ ir-sink=124:19 ir-sink=126:43 ir-sink=128:44 ast,ir=124:19 ast,ir=126:43 ast,ir=128:44
+        sink(p); // $ ast,ir-sink=124:19 ast,ir-sink=126:43 ast,ir-sink=128:44
     }
 };
 
@@ -89,41 +89,41 @@ class CRTPDoesNotCallSink : public CRTP<CRTPDoesNotCallSink> {
 };
 
 int main(int argc, char *argv[]) {
-    sink(argv[0]); // $ ast ir-path ir-sink
+    sink(argv[0]); // $ ast,ir-path,ir-sink
 
-    sink(reinterpret_cast<int>(argv)); // $ ast ir-sink
+    sink(reinterpret_cast<int>(argv)); // $ ast,ir-sink
 
-    calls_sink_with_argv(argv[1]); // $ ast ir-path=96:26 ir-path=98:18
+    calls_sink_with_argv(argv[1]); // $ ast,ir-path
 
-    char*** p = &argv; // $ ast ir-path=96:26 ir-path=98:18
+    char*** p = &argv; // $ ast,ir-path
 
-    sink(*p[0]); // $ ast ir-sink
+    sink(*p[0]); // $ ast,ir-sink
 
     calls_sink_with_argv(*p[i]); // $ MISSING: ast,ir-path
 
-    sink(*(argv + 1)); // $ ast ir-path ir-sink
+    sink(*(argv + 1)); // $ ast,ir-path ir-sink
 
     BaseWithPureVirtual* b = new DerivedCallsSink;
 
-    b->f(argv[1]); // $ ast ir-path
+    b->f(argv[1]); // $ ast,ir-path
 
     b = new DerivedDoesNotCallSink;
     b->f(argv[0]); // $ SPURIOUS: ast
 
     BaseWithPureVirtual* b2 = new DerivesMultiple;
 
-    b2->f(argv[i]); // $ ast ir-path
+    b2->f(argv[i]); // $ ast,ir-path
 
     CRTP<CRTPDoesNotCallSink> crtp_not_call_sink;
     crtp_not_call_sink.f(argv[0]); // clean
 
     CRTP<CRTPCallsSink> crtp_calls_sink;
-    crtp_calls_sink.f(argv[0]); // $ ast ir-path
+    crtp_calls_sink.f(argv[0]); // $ ast,ir-path
 
     Derived1* calls_sink = new Derived3;
-    calls_sink->f(argv[1]); // $ ast ir-path=124:19 ir-path=126:43 ir-path=128:44
+    calls_sink->f(argv[1]); // $ ast,ir-path
 
-    static_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast ir-path=124:19 ir-path=126:43 ir-path=128:44
+    static_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir-path
 
-    dynamic_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast ir-path=124:19 ir-path=126:43 ir-path=128:44
+    dynamic_cast<Derived2*>(calls_sink)->f(argv[1]); // $ ast,ir-path
 }