C++: Ensure that product dataflow library enters/leaves through the same call.

Author: MathiasVP

cpp/ql/lib/experimental/semmle/code/cpp/dataflow/ProductFlow.qll

@@ -1,4 +1,6 @@
 import semmle.code.cpp.ir.dataflow.DataFlow
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowPrivate
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowUtil
 private import codeql.util.Unit
 
 module ProductFlow {
@@ -352,32 +354,63 @@ module ProductFlow {
         pragma[only_bind_out](succ.getNode().getEnclosingCallable())
     }
 
+    newtype TKind =
+      TInto(DataFlowCall call) {
+        [any(Flow1::PathNode n).getNode(), any(Flow2::PathNode n).getNode()]
+            .(ArgumentNode)
+            .getCall() = call
+      } or
+      TOutOf(DataFlowCall call) {
+        [any(Flow1::PathNode n).getNode(), any(Flow2::PathNode n).getNode()].(OutNode).getCall() =
+          call
+      }
+
     pragma[nomagic]
     private predicate interprocEdge1(
-      Declaration predDecl, Declaration succDecl, Flow1::PathNode pred1, Flow1::PathNode succ1
+      Declaration predDecl, Declaration succDecl, Flow1::PathNode pred1, Flow1::PathNode succ1,
+      TKind kind
     ) {
       Flow1::PathGraph::edges(pred1, succ1) and
       predDecl != succDecl and
       pred1.getNode().getEnclosingCallable() = predDecl and
-      succ1.getNode().getEnclosingCallable() = succDecl
+      succ1.getNode().getEnclosingCallable() = succDecl and
+      exists(DataFlowCall call |
+        kind = TInto(call) and
+        pred1.getNode().(ArgumentNode).getCall() = call and
+        succ1.getNode() instanceof ParameterNode
+        or
+        kind = TOutOf(call) and
+        succ1.getNode().(OutNode).getCall() = call and
+        pred1.getNode() instanceof ReturnNode
+      )
     }
 
     pragma[nomagic]
     private predicate interprocEdge2(
-      Declaration predDecl, Declaration succDecl, Flow2::PathNode pred2, Flow2::PathNode succ2
+      Declaration predDecl, Declaration succDecl, Flow2::PathNode pred2, Flow2::PathNode succ2,
+      TKind kind
     ) {
       Flow2::PathGraph::edges(pred2, succ2) and
       predDecl != succDecl and
       pred2.getNode().getEnclosingCallable() = predDecl and
-      succ2.getNode().getEnclosingCallable() = succDecl
+      succ2.getNode().getEnclosingCallable() = succDecl and
+      exists(DataFlowCall call |
+        kind = TInto(call) and
+        pred2.getNode().(ArgumentNode).getCall() = call and
+        succ2.getNode() instanceof ParameterNode
+        or
+        kind = TOutOf(call) and
+        succ2.getNode().(OutNode).getCall() = call and
+        pred2.getNode() instanceof ReturnNode
+      )
     }
 
     private predicate interprocEdgePair(
       Flow1::PathNode pred1, Flow2::PathNode pred2, Flow1::PathNode succ1, Flow2::PathNode succ2
     ) {
-      exists(Declaration predDecl, Declaration succDecl |
-        interprocEdge1(predDecl, succDecl, pred1, succ1) and
-        interprocEdge2(predDecl, succDecl, pred2, succ2)
+      exists(Declaration predDecl, Declaration succDecl, TKind kind |
+        interprocEdge1(predDecl, succDecl, pred1, succ1, kind) and
+        interprocEdge2(predDecl, succDecl, pred2, succ2, kind)
       )
     }
 

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-119/OverrunWriteProductFlow.expected

@@ -398,4 +398,3 @@ subpaths
 | test.cpp:199:9:199:15 | call to strncpy | test.cpp:147:19:147:24 | call to malloc | test.cpp:199:22:199:27 | string | This write may overflow $@ by 2 elements. | test.cpp:199:22:199:27 | string | string |
 | test.cpp:203:9:203:15 | call to strncpy | test.cpp:147:19:147:24 | call to malloc | test.cpp:203:22:203:27 | string | This write may overflow $@ by 2 elements. | test.cpp:203:22:203:27 | string | string |
 | test.cpp:207:9:207:15 | call to strncpy | test.cpp:147:19:147:24 | call to malloc | test.cpp:207:22:207:27 | string | This write may overflow $@ by 3 elements. | test.cpp:207:22:207:27 | string | string |
-| test.cpp:216:3:216:8 | call to memset | test.cpp:220:43:220:48 | call to malloc | test.cpp:216:10:216:10 | p | This write may overflow $@ by 5 elements. | test.cpp:216:10:216:10 | p | p |