Merge pull request github#10505 from hvitved/dataflow/viable-impl-in-ctx-consistency

Data flow: Guard against `viableImplInCallContext` not being a subset of `viableCallable`

Author: hvitved

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll

@@ -709,7 +709,8 @@ private module Cached {
      */
     pragma[nomagic]
     private DataFlowCallable viableImplInCallContextExt(DataFlowCall call, DataFlowCall ctx) {
-      result = viableImplInCallContext(call, ctx)
+      result = viableImplInCallContext(call, ctx) and
+      result = viableCallable(call)
       or
       result = viableCallableLambda(call, TDataFlowCallSome(ctx))
       or

cpp/ql/lib/experimental/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll

@@ -38,6 +38,13 @@ module Consistency {
 
     /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */
     predicate uniquePostUpdateExclude(Node n) { none() }
+
+    /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */
+    predicate viableImplInCallContextTooLargeExclude(
+      DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+    ) {
+      none()
+    }
   }
 
   private class RelevantNode extends Node {
@@ -217,4 +224,12 @@ module Consistency {
     not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
     msg = "PostUpdateNode should not be the target of local flow."
   }
+
+  query predicate viableImplInCallContextTooLarge(
+    DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+  ) {
+    callable = viableImplInCallContext(call, ctx) and
+    not callable = viableCallable(call) and
+    not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
+  }
 }

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplCommon.qll

@@ -709,7 +709,8 @@ private module Cached {
      */
     pragma[nomagic]
     private DataFlowCallable viableImplInCallContextExt(DataFlowCall call, DataFlowCall ctx) {
-      result = viableImplInCallContext(call, ctx)
+      result = viableImplInCallContext(call, ctx) and
+      result = viableCallable(call)
       or
       result = viableCallableLambda(call, TDataFlowCallSome(ctx))
       or

cpp/ql/lib/semmle/code/cpp/dataflow/internal/DataFlowImplConsistency.qll

@@ -38,6 +38,13 @@ module Consistency {
 
     /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */
     predicate uniquePostUpdateExclude(Node n) { none() }
+
+    /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */
+    predicate viableImplInCallContextTooLargeExclude(
+      DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+    ) {
+      none()
+    }
   }
 
   private class RelevantNode extends Node {
@@ -217,4 +224,12 @@ module Consistency {
     not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
     msg = "PostUpdateNode should not be the target of local flow."
   }
+
+  query predicate viableImplInCallContextTooLarge(
+    DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+  ) {
+    callable = viableImplInCallContext(call, ctx) and
+    not callable = viableCallable(call) and
+    not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
+  }
 }

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplCommon.qll

@@ -709,7 +709,8 @@ private module Cached {
      */
     pragma[nomagic]
     private DataFlowCallable viableImplInCallContextExt(DataFlowCall call, DataFlowCall ctx) {
-      result = viableImplInCallContext(call, ctx)
+      result = viableImplInCallContext(call, ctx) and
+      result = viableCallable(call)
       or
       result = viableCallableLambda(call, TDataFlowCallSome(ctx))
       or

cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowImplConsistency.qll

@@ -38,6 +38,13 @@ module Consistency {
 
     /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */
     predicate uniquePostUpdateExclude(Node n) { none() }
+
+    /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */
+    predicate viableImplInCallContextTooLargeExclude(
+      DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+    ) {
+      none()
+    }
   }
 
   private class RelevantNode extends Node {
@@ -217,4 +224,12 @@ module Consistency {
     not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
     msg = "PostUpdateNode should not be the target of local flow."
   }
+
+  query predicate viableImplInCallContextTooLarge(
+    DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+  ) {
+    callable = viableImplInCallContext(call, ctx) and
+    not callable = viableCallable(call) and
+    not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
+  }
 }

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

@@ -87,3 +87,4 @@ postWithInFlow
 | test.cpp:465:3:465:4 | * ... [post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:465:4:465:4 | p [inner post update] | PostUpdateNode should not be the target of local flow. |
 | test.cpp:470:22:470:22 | x [inner post update] | PostUpdateNode should not be the target of local flow. |
+viableImplInCallContextTooLarge

cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-ir-consistency.expected

@@ -627,3 +627,4 @@ postWithInFlow
 | true_upon_entry.cpp:98:7:98:7 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
 | true_upon_entry.cpp:101:18:101:18 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
 | true_upon_entry.cpp:102:5:102:5 | x [post update] | PostUpdateNode should not be the target of local flow. |
+viableImplInCallContextTooLarge

cpp/ql/test/library-tests/dataflow/fields/dataflow-consistency.expected

@@ -155,3 +155,4 @@ postWithInFlow
 | simple.cpp:92:7:92:7 | i [post update] | PostUpdateNode should not be the target of local flow. |
 | struct_init.c:24:11:24:12 | ab [inner post update] | PostUpdateNode should not be the target of local flow. |
 | struct_init.c:36:17:36:24 | nestedAB [inner post update] | PostUpdateNode should not be the target of local flow. |
+viableImplInCallContextTooLarge

cpp/ql/test/library-tests/dataflow/fields/dataflow-ir-consistency.expected

@@ -1323,3 +1323,4 @@ postWithInFlow
 | struct_init.c:46:16:46:24 | FieldAddress [post update] | PostUpdateNode should not be the target of local flow. |
 | struct_init.c:46:16:46:24 | pointerAB [post update] | PostUpdateNode should not be the target of local flow. |
 | struct_init.c:46:16:46:24 | pointerAB [post update] | PostUpdateNode should not be the target of local flow. |
+viableImplInCallContextTooLarge

cpp/ql/test/library-tests/syntax-zoo/dataflow-consistency.expected

@@ -124,3 +124,4 @@ postWithInFlow
 | static_init_templates.cpp:3:2:3:4 | ref [post update] | PostUpdateNode should not be the target of local flow. |
 | static_init_templates.cpp:21:2:21:4 | val [post update] | PostUpdateNode should not be the target of local flow. |
 | try_catch.cpp:7:8:7:8 | call to exception | PostUpdateNode should not be the target of local flow. |
+viableImplInCallContextTooLarge

cpp/ql/test/library-tests/syntax-zoo/dataflow-ir-consistency.expected

@@ -2710,3 +2710,4 @@ postWithInFlow
 | whilestmt.c:11:5:11:8 | done [post update] | PostUpdateNode should not be the target of local flow. |
 | whilestmt.c:40:7:40:7 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
 | whilestmt.c:42:7:42:7 | VariableAddress [post update] | PostUpdateNode should not be the target of local flow. |
+viableImplInCallContextTooLarge

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowDispatch.qll

@@ -175,10 +175,19 @@ private module DispatchImpl {
    * restricted to those `call`s for which a context might make a difference.
    */
   DataFlowCallable viableImplInCallContext(NonDelegateDataFlowCall call, DataFlowCall ctx) {
-    result.getUnderlyingCallable() =
-      call.getDispatchCall()
-          .getADynamicTargetInCallContext(ctx.(NonDelegateDataFlowCall).getDispatchCall())
-          .getUnboundDeclaration()
+    exists(DispatchCall dc | dc = call.getDispatchCall() |
+      result.getUnderlyingCallable() =
+        getCallableForDataFlow(dc.getADynamicTargetInCallContext(ctx.(NonDelegateDataFlowCall)
+                .getDispatchCall()).getUnboundDeclaration())
+      or
+      exists(Callable c, DataFlowCallable encl |
+        result.asSummarizedCallable() = c and
+        mayBenefitFromCallContext(call, encl) and
+        encl = ctx.getARuntimeTarget() and
+        c = dc.getAStaticTarget().getUnboundDeclaration() and
+        not c instanceof RuntimeCallable
+      )
+    )
   }
 }
 

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplCommon.qll

@@ -709,7 +709,8 @@ private module Cached {
      */
     pragma[nomagic]
     private DataFlowCallable viableImplInCallContextExt(DataFlowCall call, DataFlowCall ctx) {
-      result = viableImplInCallContext(call, ctx)
+      result = viableImplInCallContext(call, ctx) and
+      result = viableCallable(call)
       or
       result = viableCallableLambda(call, TDataFlowCallSome(ctx))
       or

csharp/ql/lib/semmle/code/csharp/dataflow/internal/DataFlowImplConsistency.qll

@@ -38,6 +38,13 @@ module Consistency {
 
     /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */
     predicate uniquePostUpdateExclude(Node n) { none() }
+
+    /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */
+    predicate viableImplInCallContextTooLargeExclude(
+      DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+    ) {
+      none()
+    }
   }
 
   private class RelevantNode extends Node {
@@ -217,4 +224,12 @@ module Consistency {
     not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
     msg = "PostUpdateNode should not be the target of local flow."
   }
+
+  query predicate viableImplInCallContextTooLarge(
+    DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+  ) {
+    callable = viableImplInCallContext(call, ctx) and
+    not callable = viableCallable(call) and
+    not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
+  }
 }

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplCommon.qll

@@ -709,7 +709,8 @@ private module Cached {
      */
     pragma[nomagic]
     private DataFlowCallable viableImplInCallContextExt(DataFlowCall call, DataFlowCall ctx) {
-      result = viableImplInCallContext(call, ctx)
+      result = viableImplInCallContext(call, ctx) and
+      result = viableCallable(call)
       or
       result = viableCallableLambda(call, TDataFlowCallSome(ctx))
       or

java/ql/lib/semmle/code/java/dataflow/internal/DataFlowImplConsistency.qll

@@ -38,6 +38,13 @@ module Consistency {
 
     /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */
     predicate uniquePostUpdateExclude(Node n) { none() }
+
+    /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */
+    predicate viableImplInCallContextTooLargeExclude(
+      DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+    ) {
+      none()
+    }
   }
 
   private class RelevantNode extends Node {
@@ -217,4 +224,12 @@ module Consistency {
     not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
     msg = "PostUpdateNode should not be the target of local flow."
   }
+
+  query predicate viableImplInCallContextTooLarge(
+    DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+  ) {
+    callable = viableImplInCallContext(call, ctx) and
+    not callable = viableCallable(call) and
+    not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
+  }
 }

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplCommon.qll

@@ -709,7 +709,8 @@ private module Cached {
      */
     pragma[nomagic]
     private DataFlowCallable viableImplInCallContextExt(DataFlowCall call, DataFlowCall ctx) {
-      result = viableImplInCallContext(call, ctx)
+      result = viableImplInCallContext(call, ctx) and
+      result = viableCallable(call)
       or
       result = viableCallableLambda(call, TDataFlowCallSome(ctx))
       or

python/ql/lib/semmle/python/dataflow/new/internal/DataFlowImplConsistency.qll

@@ -38,6 +38,13 @@ module Consistency {
 
     /** Holds if `n` should be excluded from the consistency test `uniquePostUpdate`. */
     predicate uniquePostUpdateExclude(Node n) { none() }
+
+    /** Holds if `(call, ctx)` should be excluded from the consistency test `viableImplInCallContextTooLargeExclude`. */
+    predicate viableImplInCallContextTooLargeExclude(
+      DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+    ) {
+      none()
+    }
   }
 
   private class RelevantNode extends Node {
@@ -217,4 +224,12 @@ module Consistency {
     not any(ConsistencyConfiguration c).postWithInFlowExclude(n) and
     msg = "PostUpdateNode should not be the target of local flow."
   }
+
+  query predicate viableImplInCallContextTooLarge(
+    DataFlowCall call, DataFlowCall ctx, DataFlowCallable callable
+  ) {
+    callable = viableImplInCallContext(call, ctx) and
+    not callable = viableCallable(call) and
+    not any(ConsistencyConfiguration c).viableImplInCallContextTooLargeExclude(call, ctx, callable)
+  }
 }

python/ql/test/experimental/dataflow/basic/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/calls/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/consistency/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/coverage/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/fieldflow/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/global-flow/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/match/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/pep_328/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/regression/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/strange-essaflow/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/tainttracking/basic/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/tainttracking/commonSanitizer/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/tainttracking/customSanitizer/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep-py3/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/tainttracking/defaultAdditionalTaintStep/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/tainttracking/unwanted-global-flow/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/typetracking/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/experimental/dataflow/variable-capture/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/library-tests/ApiGraphs/py3/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

python/ql/test/library-tests/frameworks/django-orm/dataflow-consistency.expected

@@ -16,3 +16,4 @@ postIsInSameCallable
 reverseRead
 argHasPostUpdate
 postWithInFlow
+viableImplInCallContextTooLarge

ruby/ql/lib/codeql/ruby/dataflow/internal/DataFlowImplCommon.qll

@@ -709,7 +709,8 @@ private module Cached {
      */
     pragma[nomagic]
     private DataFlowCallable viableImplInCallContextExt(DataFlowCall call, DataFlowCall ctx) {
-      result = viableImplInCallContext(call, ctx)
+      result = viableImplInCallContext(call, ctx) and
+      result = viableCallable(call)
       or
       result = viableCallableLambda(call, TDataFlowCallSome(ctx))
       or