Merge pull request #19569 from MathiasVP/fix-return-value-surces-in-mad

Shared/C++: Handle non-standard return values in MaD flow sources/sinks

Author: MathiasVP

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

@@ -22,7 +22,11 @@ module Input implements InputSig<Location, DataFlowImplSpecific::CppDataFlow> {
 
   ArgumentPosition callbackSelfParameterPosition() { result = TDirectPosition(-1) }
 
-  ReturnKind getStandardReturnValueKind() { result.(NormalReturnKind).getIndirectionIndex() = 0 }
+  ReturnKind getStandardReturnValueKind() { result = getReturnValueKind("") }
+
+  ReturnKind getReturnValueKind(string arg) {
+    arg = repeatStars(result.(NormalReturnKind).getIndirectionIndex())
+  }
 
   string encodeParameterPosition(ParameterPosition pos) { result = pos.toString() }
 

cpp/ql/test/library-tests/dataflow/models-as-data/tests.cpp

@@ -56,9 +56,9 @@ void test_sources() {
 	sink(v_direct); // $ ir
 
 	sink(remoteMadSourceIndirect());
-	sink(*remoteMadSourceIndirect()); // $ MISSING: ir
+	sink(*remoteMadSourceIndirect()); // $ ir
 	sink(*remoteMadSourceDoubleIndirect());
-	sink(**remoteMadSourceDoubleIndirect()); // $ MISSING: ir
+	sink(**remoteMadSourceDoubleIndirect()); // $ ir
 
 	int a, b, c, d;
 
@@ -124,7 +124,7 @@ void test_sinks() {
 	// test sources + sinks together
 
 	madSinkArg0(localMadSource()); // $ ir
-	madSinkIndirectArg0(remoteMadSourceIndirect()); // $ MISSING: ir
+	madSinkIndirectArg0(remoteMadSourceIndirect()); // $ ir
 	madSinkVar = remoteMadSourceVar; // $ ir
 	*madSinkVarIndirect = remoteMadSourceVar; // $ MISSING: ir
 }

shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll

@@ -54,6 +54,20 @@ signature module InputSig<LocationSig Location, DF::InputSig<Location> Lang> {
   /** Gets the return kind corresponding to specification `"ReturnValue"`. */
   Lang::ReturnKind getStandardReturnValueKind();
 
+  /**
+   * Gets the return kind corresponding to specification `"ReturnValue"` when
+   * supplied with the argument `arg`.
+   *
+   * Note that it is expected that the following equality holds:
+   * ```
+   * getReturnValueKind("") = getStandardReturnValueKind()
+   * ```
+   */
+  default Lang::ReturnKind getReturnValueKind(string arg) {
+    arg = "" and
+    result = getStandardReturnValueKind()
+  }
+
   /** Gets the textual representation of parameter position `pos` used in MaD. */
   string encodeParameterPosition(Lang::ParameterPosition pos);
 
@@ -2164,9 +2178,15 @@ module Make<
               )
             )
             or
-            c = "ReturnValue" and
-            node.asNode() =
-              getAnOutNodeExt(mid.asCall(), TValueReturn(getStandardReturnValueKind()))
+            c.getName() = "ReturnValue" and
+            exists(ReturnKind rk |
+              not exists(c.getAnArgument()) and
+              rk = getStandardReturnValueKind()
+              or
+              rk = getReturnValueKind(c.getAnArgument())
+            |
+              node.asNode() = getAnOutNodeExt(mid.asCall(), TValueReturn(rk))
+            )
             or
             SourceSinkInterpretationInput::interpretOutput(c, mid, node)
           )
@@ -2198,12 +2218,16 @@ module Make<
               )
             )
             or
-            exists(ReturnNode ret, ValueReturnKind kind |
-              c = "ReturnValue" and
+            exists(ReturnNode ret, ReturnKind kind |
+              c.getName() = "ReturnValue" and
               ret = node.asNode() and
-              kind.getKind() = ret.getKind() and
-              kind.getKind() = getStandardReturnValueKind() and
+              kind = ret.getKind() and
               mid.asCallable() = getNodeEnclosingCallable(ret)
+            |
+              not exists(c.getAnArgument()) and
+              kind = getStandardReturnValueKind()
+              or
+              kind = getReturnValueKind(c.getAnArgument())
             )
             or
             SourceSinkInterpretationInput::interpretInput(c, mid, node)