From 9ea4c40ca999b66bba411c28e96114bc39c09f50 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Oct 2024 14:13:55 +0100 Subject: [PATCH 1/6] C++: Add failing test. --- .../dataflow-tests/dataflow-consistency.expected | 3 +++ .../dataflow/dataflow-tests/test.cpp | 16 ++++++++++++++++ 2 files changed, 19 insertions(+) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected index d97abde482eb..e9dc3cdb790b 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/dataflow-consistency.expected @@ -3,6 +3,9 @@ uniqueEnclosingCallable | test.cpp:864:47:864:54 | call to source | Node should have one enclosing callable but has 0. | | test.cpp:872:46:872:51 | call to source | Node should have one enclosing callable but has 0. | | test.cpp:872:53:872:56 | 1 | Node should have one enclosing callable but has 0. | +| test.cpp:1126:33:1129:1 | {...} | Node should have one enclosing callable but has 0. | +| test.cpp:1127:3:1127:13 | reads_input | Node should have one enclosing callable but has 0. | +| test.cpp:1128:3:1128:21 | not_does_read_input | Node should have one enclosing callable but has 0. | uniqueCallEnclosingCallable | test.cpp:864:47:864:54 | call to source | Call should have one enclosing callable but has 0. | | test.cpp:872:46:872:51 | call to source | Call should have one enclosing callable but has 0. | diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index 3a6ffe9b716b..4c84a209b4f7 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -1115,4 +1115,20 @@ void indirect_sink_const_ref(const T&); void test_temp_with_conversion_from_materialization() { indirect_sink_const_ref(source()); // $ ir MISSING: ast +} + +void reads_input(int x) { + sink(x); // $ MISSING: ast,ir +} + +void not_does_read_input(int x); + +void (*dispatch_table[])(int) = { + reads_input, + not_does_read_input +}; + +void test_dispatch_table(int i) { + int x = source(); + dispatch_table[i](x); } \ No newline at end of file From 30e07817781f06efc88ddd554bde213b86fa7e5d Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Oct 2024 14:14:52 +0100 Subject: [PATCH 2/6] C++: Also check for source calls when using 'lambda call resolution'. --- .../semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll index ac6e898748ab..8f0ae53171e3 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll @@ -1328,7 +1328,10 @@ predicate lambdaCreation(Node creation, LambdaCallKind kind, DataFlowCallable c) /** Holds if `call` is a lambda call of kind `kind` where `receiver` is the lambda expression. */ predicate lambdaCall(DataFlowCall call, LambdaCallKind kind, Node receiver) { - call.(SummaryCall).getReceiver() = receiver.(FlowSummaryNode).getSummaryNode() and + ( + call.(SummaryCall).getReceiver() = receiver.(FlowSummaryNode).getSummaryNode() or + call.asCallInstruction().getCallTargetOperand() = receiver.asOperand() + ) and exists(kind) } From 2dbf75fde91e64a9169f3cbf6042fc7d48b2c14f Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Oct 2024 14:15:05 +0100 Subject: [PATCH 3/6] C++: Accept test changes. --- .../dataflow/dataflow-tests/test-source-sink.expected | 1 + cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected index 90f797429c96..10a8bef9a338 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test-source-sink.expected @@ -323,6 +323,7 @@ irFlow | test.cpp:1069:9:1069:14 | call to source | test.cpp:1074:10:1074:10 | i | | test.cpp:1069:9:1069:14 | call to source | test.cpp:1081:10:1081:10 | i | | test.cpp:1117:27:1117:34 | call to source | test.cpp:1117:27:1117:34 | call to source | +| test.cpp:1132:11:1132:16 | call to source | test.cpp:1121:8:1121:8 | x | | true_upon_entry.cpp:9:11:9:16 | call to source | true_upon_entry.cpp:13:8:13:8 | x | | true_upon_entry.cpp:17:11:17:16 | call to source | true_upon_entry.cpp:21:8:21:8 | x | | true_upon_entry.cpp:27:9:27:14 | call to source | true_upon_entry.cpp:29:8:29:8 | x | diff --git a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp index 4c84a209b4f7..60baa08bb8d7 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -1118,7 +1118,7 @@ void test_temp_with_conversion_from_materialization() { } void reads_input(int x) { - sink(x); // $ MISSING: ast,ir + sink(x); // $ ir MISSING: ast } void not_does_read_input(int x); From a99d57640a2b08d3f775539f9317406b737120a8 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Oct 2024 14:45:44 +0100 Subject: [PATCH 4/6] C++: Add a new API for getting the target of a 'Call' expression. --- .../code/cpp/ir/dataflow/internal/DataFlowUtil.qll | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll index f2263abf7f52..d0935bb76d2e 100644 --- a/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll +++ b/cpp/ql/lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll @@ -17,6 +17,7 @@ private import SsaInternals as Ssa private import DataFlowImplCommon as DataFlowImplCommon private import codeql.util.Unit private import Node0ToString +private import DataFlowDispatch as DataFlowDispatch import ExprNodes /** @@ -2497,3 +2498,16 @@ class AdditionalCallTarget extends Unit { */ abstract Declaration viableTarget(Call call); } + +/** + * Gets a function that may be called by `call`. + * + * Note that `call` may be a call to a function pointer expression. + */ +Function getARuntimeTarget(Call call) { + exists(DataFlowCall dfCall | dfCall.asCallInstruction().getUnconvertedResultExpression() = call | + result = DataFlowDispatch::viableCallable(dfCall).asSourceCallable() + or + result = DataFlowImplCommon::viableCallableLambda(dfCall, _).asSourceCallable() + ) +} From baab74cb3524733f6a561bfd332d17e50333cc62 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Wed, 16 Oct 2024 17:45:44 +0100 Subject: [PATCH 5/6] C++: Add change notes. --- .../change-notes/2024-10-16-function-pointer-resolution.md | 4 ++++ .../2024-10-16-new-api-for-call-target-resolution.md | 4 ++++ 2 files changed, 8 insertions(+) create mode 100644 cpp/ql/lib/change-notes/2024-10-16-function-pointer-resolution.md create mode 100644 cpp/ql/lib/change-notes/2024-10-16-new-api-for-call-target-resolution.md diff --git a/cpp/ql/lib/change-notes/2024-10-16-function-pointer-resolution.md b/cpp/ql/lib/change-notes/2024-10-16-function-pointer-resolution.md new file mode 100644 index 000000000000..d682ee78d592 --- /dev/null +++ b/cpp/ql/lib/change-notes/2024-10-16-function-pointer-resolution.md @@ -0,0 +1,4 @@ +--- +category: minorAnalysis +--- +* The function call target resolution algorithm has been improved to resolve more calls through function pointers. As a result, dataflow queries may have more results. \ No newline at end of file diff --git a/cpp/ql/lib/change-notes/2024-10-16-new-api-for-call-target-resolution.md b/cpp/ql/lib/change-notes/2024-10-16-new-api-for-call-target-resolution.md new file mode 100644 index 000000000000..2e1898845f3c --- /dev/null +++ b/cpp/ql/lib/change-notes/2024-10-16-new-api-for-call-target-resolution.md @@ -0,0 +1,4 @@ +--- +category: feature +--- +* Added a new predicate `DataFlow::getARuntimeTarget` for getting a function that may be invoked by a `Call` expression. Unlike `Call.getTarget` this new predicate may also resolves function pointers. \ No newline at end of file From 5e04358ece4aeea08c2ed8b740aa6f3c08f9fccb Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Thu, 17 Oct 2024 10:57:30 +0100 Subject: [PATCH 6/6] Update cpp/ql/lib/change-notes/2024-10-16-new-api-for-call-target-resolution.md Co-authored-by: Jeroen Ketema <93738568+jketema@users.noreply.github.com> --- .../2024-10-16-new-api-for-call-target-resolution.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cpp/ql/lib/change-notes/2024-10-16-new-api-for-call-target-resolution.md b/cpp/ql/lib/change-notes/2024-10-16-new-api-for-call-target-resolution.md index 2e1898845f3c..db4da19c02b5 100644 --- a/cpp/ql/lib/change-notes/2024-10-16-new-api-for-call-target-resolution.md +++ b/cpp/ql/lib/change-notes/2024-10-16-new-api-for-call-target-resolution.md @@ -1,4 +1,4 @@ --- category: feature --- -* Added a new predicate `DataFlow::getARuntimeTarget` for getting a function that may be invoked by a `Call` expression. Unlike `Call.getTarget` this new predicate may also resolves function pointers. \ No newline at end of file +* Added a new predicate `DataFlow::getARuntimeTarget` for getting a function that may be invoked by a `Call` expression. Unlike `Call.getTarget` this new predicate may also resolve function pointers. \ No newline at end of file