Merge pull request #2678 from MathiasVP/union-access-global-virtual-dispatch

C++: IR virtual dispatch through union field access

Author: jbj

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

@@ -83,10 +83,24 @@ private module VirtualDispatch {
       )
       or
       // Flow through global variable
-      exists(StoreInstruction store, Variable var |
+      exists(StoreInstruction store |
         store = src.asInstruction() and
-        var = store.getDestinationAddress().(VariableAddressInstruction).getASTVariable() and
-        this.flowsFromGlobal(var) and
+        (
+          exists(Variable var |
+            var = store.getDestinationAddress().(VariableAddressInstruction).getASTVariable() and
+            this.flowsFromGlobal(var)
+          )
+          or
+          exists(Variable var, FieldAccess a |
+            var = store
+                  .getDestinationAddress()
+                  .(FieldAddressInstruction)
+                  .getObjectAddress()
+                  .(VariableAddressInstruction)
+                  .getASTVariable() and
+            this.flowsFromGlobalUnionField(var, a)
+          )
+        ) and
         allowFromArg = true
       )
     }
@@ -97,6 +111,19 @@ private module VirtualDispatch {
         load.getSourceAddress().(VariableAddressInstruction).getASTVariable() = var
       )
     }
+
+    private predicate flowsFromGlobalUnionField(Variable var, FieldAccess a) {
+      a.getTarget().getDeclaringType() instanceof Union and
+      exists(LoadInstruction load |
+        this.flowsFrom(DataFlow::instructionNode(load), _) and
+        load
+            .getSourceAddress()
+            .(FieldAddressInstruction)
+            .getObjectAddress()
+            .(VariableAddressInstruction)
+            .getASTVariable() = var
+      )
+    }
   }
 
   /** Call through a function pointer. */

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

@@ -130,3 +130,46 @@ namespace virtual_inheritance {
     sink(topRef.isSource()); // flow [NOT DETECTED]
   }
 }
+
+union union_with_sink_fun_ptrs {
+  SinkFunctionType f;
+  SinkFunctionType g;
+} u;
+
+void call_sink_through_union_field_f(SinkFunctionType func) {
+  func(source());
+}
+
+void call_sink_through_union_field_g(SinkFunctionType func) {
+  func(source());
+}
+
+void set_global_union_field_f() {
+  u.f = callSink;
+}
+
+void test_call_sink_through_union() {
+  set_global_union_field_f();
+  call_sink_through_union_field_f(u.f);
+  call_sink_through_union_field_g(u.g);
+}
+
+union { union_with_sink_fun_ptrs u; } u2;
+
+void call_sink_through_union_field_u_g(SinkFunctionType func) {
+  func(source());
+}
+
+void call_sink_through_union_field_u_f(SinkFunctionType func) {
+  func(source());
+}
+
+void set_global_union_field_u_f() {
+  u2.u.f = callSink;
+}
+
+void test_call_sink_through_union_2() {
+  set_global_union_field_u_f();
+  call_sink_through_union_field_u_f(u2.u.f); // flow [NOT DETECTED]
+  call_sink_through_union_field_u_g(u2.u.g); // flow [NOT DETECTED]
+}

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

@@ -18,6 +18,8 @@
 | dispatch.cpp:73:14:73:19 | dispatch.cpp:23:38:23:38 | IR only |
 | dispatch.cpp:81:13:81:18 | dispatch.cpp:23:38:23:38 | IR only |
 | dispatch.cpp:107:17:107:22 | dispatch.cpp:96:8:96:8 | IR only |
+| dispatch.cpp:140:8:140:13 | dispatch.cpp:96:8:96:8 | IR only |
+| dispatch.cpp:144:8:144:13 | dispatch.cpp:96:8:96:8 | IR only |
 | lambdas.cpp:8:10:8:15 | lambdas.cpp:14:3:14:6 | AST only |
 | lambdas.cpp:8:10:8:15 | lambdas.cpp:18:8:18:8 | AST only |
 | lambdas.cpp:8:10:8:15 | lambdas.cpp:21:3:21:6 | AST only |

cpp/ql/test/library-tests/dataflow/dataflow-tests/test_ir.expected

@@ -28,6 +28,8 @@
 | dispatch.cpp:55:22:55:30 | call to isSource1 | dispatch.cpp:22:37:22:42 | call to source |
 | dispatch.cpp:58:28:58:36 | call to isSource1 | dispatch.cpp:22:37:22:42 | call to source |
 | dispatch.cpp:96:8:96:8 | x | dispatch.cpp:107:17:107:22 | call to source |
+| dispatch.cpp:96:8:96:8 | x | dispatch.cpp:140:8:140:13 | call to source |
+| dispatch.cpp:96:8:96:8 | x | dispatch.cpp:144:8:144:13 | call to source |
 | test.cpp:7:8:7:9 | t1 | test.cpp:6:12:6:17 | call to source |
 | test.cpp:9:8:9:9 | t1 | test.cpp:6:12:6:17 | call to source |
 | test.cpp:10:8:10:9 | t2 | test.cpp:6:12:6:17 | call to source |