Merge pull request #2262 from jbj/ir-virtual-dispatch-local

C++: Rudimentary support for IR data flow virtual dispatch

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

@@ -2,6 +2,7 @@ import cpp
 import semmle.code.cpp.security.Security
 private import semmle.code.cpp.ir.dataflow.DataFlow
 private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.dataflow.internal.DataFlowDispatch as Dispatch
 
 /**
  * A predictable instruction is one where an external user can predict
@@ -145,7 +146,8 @@ GlobalOrNamespaceVariable globalVarFromId(string id) {
 }
 
 Function resolveCall(Call call) {
-  // TODO: improve virtual dispatch. This will help in the test for
-  // `UncontrolledProcessOperation.ql`.
-  result = call.getTarget()
+  exists(CallInstruction callInstruction |
+    callInstruction.getAST() = call and
+    result = Dispatch::viableCallable(callInstruction)
+  )
 }

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

@@ -1,5 +1,6 @@
 private import cpp
 private import semmle.code.cpp.ir.IR
+private import semmle.code.cpp.ir.dataflow.DataFlow
 
 Function viableImpl(CallInstruction call) { result = viableCallable(call) }
 
@@ -20,6 +21,58 @@ Function viableCallable(CallInstruction call) {
     functionSignatureWithBody(qualifiedName, nparams, result) and
     strictcount(Function other | functionSignatureWithBody(qualifiedName, nparams, other)) = 1
   )
+  or
+  // Rudimentary virtual dispatch support. It's essentially local data flow
+  // where the source is a derived-to-base conversion and the target is the
+  // qualifier of a call.
+  exists(Class derived, DataFlow::Node thisArgument |
+    nodeMayHaveClass(derived, thisArgument) and
+    overrideMayAffectCall(derived, thisArgument, _, result, call)
+  )
+}
+
+/**
+ * Holds if `call` is a virtual function call with qualifier `thisArgument` in
+ * `enclosingFunction`, whose static target is overridden by
+ * `overridingFunction` in `overridingClass`.
+ */
+pragma[noinline]
+private predicate overrideMayAffectCall(
+  Class overridingClass, DataFlow::Node thisArgument, Function enclosingFunction,
+  MemberFunction overridingFunction, CallInstruction call
+) {
+  call.getEnclosingFunction() = enclosingFunction and
+  overridingFunction.getAnOverriddenFunction+() = call.getStaticCallTarget() and
+  overridingFunction.getDeclaringType() = overridingClass and
+  thisArgument = DataFlow::instructionNode(call.getThisArgument())
+}
+
+/**
+ * Holds if `node` may have dynamic class `derived`, where `derived` is a class
+ * that may affect virtual dispatch within the enclosing function.
+ *
+ * For the sake of performance, this recursion is written out manually to make
+ * it a relation on `Class x Node` rather than `Node x Node` or `MemberFunction
+ * x Node`, both of which would be larger. It's a forward search since there
+ * should usually be fewer classes than calls.
+ *
+ * If a value is cast several classes up in the hierarchy, that will be modeled
+ * as a chain of `ConvertToBaseInstruction`s and will cause the search to start
+ * from each of them and pass through subsequent ones. There might be
+ * performance to gain by stopping before a second upcast and reconstructing
+ * the full chain in a "big-step" recursion after this one.
+ */
+private predicate nodeMayHaveClass(Class derived, DataFlow::Node node) {
+  exists(ConvertToBaseInstruction toBase |
+    derived = toBase.getDerivedClass() and
+    overrideMayAffectCall(derived, _, toBase.getEnclosingFunction(), _, _) and
+    node.asInstruction() = toBase
+  )
+  or
+  exists(DataFlow::Node prev |
+    nodeMayHaveClass(derived, prev) and
+    DataFlow::localFlowStep(prev, node)
+  )
 }
 
 /**

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

@@ -205,7 +205,8 @@ private predicate simpleInstructionLocalFlowStep(Instruction iFrom, Instruction
   iTo.(CopyInstruction).getSourceValue() = iFrom or
   iTo.(PhiInstruction).getAnOperand().getDef() = iFrom or
   // Treat all conversions as flow, even conversions between different numeric types.
-  iTo.(ConvertInstruction).getUnary() = iFrom
+  iTo.(ConvertInstruction).getUnary() = iFrom or
+  iTo.(InheritanceConversionInstruction).getUnary() = iFrom
 }
 
 /**