From 69ea19cb8b30b8bc61e0961c563fa99b6af6094d Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 23 May 2025 11:16:29 +0100 Subject: [PATCH 1/4] Shared: Add a 'getReturnValueKind' predicate and use it in 'interpretOutput' and 'interpretInput' to handle non-standard return value input/output. This is needed to support C++'s ReturnValue[**] notation. --- .../dataflow/internal/FlowSummaryImpl.qll | 38 +++++++++++++++---- 1 file changed, 31 insertions(+), 7 deletions(-) diff --git a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll index 95d29153f47e..d9efa7a84497 100644 --- a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll @@ -54,6 +54,20 @@ signature module InputSig Lang> { /** Gets the return kind corresponding to specification `"ReturnValue"`. */ Lang::ReturnKind getStandardReturnValueKind(); + /** + * Gets the return kind corresponding to specification `"ReturnValue"` when + * the supplied 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) From 07c4eca4d82a7bdcadffe805d3043a47ee7731e4 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 23 May 2025 11:16:49 +0100 Subject: [PATCH 2/4] C++: Implement the new predicate for C++. --- .../semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll index 5ba1db044290..58c3dfdea16b 100644 --- a/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll +++ b/cpp/ql/lib/semmle/code/cpp/dataflow/internal/FlowSummaryImpl.qll @@ -22,7 +22,11 @@ module Input implements InputSig { 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() } From cf39103df37ac9b19f6df084a78c4233ef5bdd17 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 23 May 2025 11:19:25 +0100 Subject: [PATCH 3/4] C++: Accept test changes. --- cpp/ql/test/library-tests/dataflow/models-as-data/tests.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/cpp/ql/test/library-tests/dataflow/models-as-data/tests.cpp b/cpp/ql/test/library-tests/dataflow/models-as-data/tests.cpp index 9f01f0959c00..92397bc91c34 100644 --- a/cpp/ql/test/library-tests/dataflow/models-as-data/tests.cpp +++ b/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 } From 92e0b6430741a0f9b217e711d8fdab553494468b Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 23 May 2025 11:43:27 +0100 Subject: [PATCH 4/4] Shared: Fix QLDoc. --- shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll index d9efa7a84497..244cc5731976 100644 --- a/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll +++ b/shared/dataflow/codeql/dataflow/internal/FlowSummaryImpl.qll @@ -56,7 +56,7 @@ signature module InputSig Lang> { /** * Gets the return kind corresponding to specification `"ReturnValue"` when - * the supplied argument `arg`. + * supplied with the argument `arg`. * * Note that it is expected that the following equality holds: * ```