From 9970f7837bf9c8a06b56408d6eb80ae9d03c1661 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 17 Jan 2025 21:03:44 +0000 Subject: [PATCH 1/4] C++: Add pointer/pointee conflation test. --- .../dataflow-tests/dataflow-consistency.expected | 2 ++ .../dataflow-tests/test-source-sink.expected | 1 + .../dataflow/dataflow-tests/test.cpp | 15 +++++++++++++++ 3 files changed, 18 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 ca62e5c92690..9492b7dd2760 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 @@ -185,6 +185,8 @@ postWithInFlow | test.cpp:1138:5:1138:8 | data [inner post update] | PostUpdateNode should not be the target of local flow. | | test.cpp:1139:3:1139:7 | * ... [post update] | PostUpdateNode should not be the target of local flow. | | test.cpp:1139:4:1139:7 | data [inner post update] | PostUpdateNode should not be the target of local flow. | +| test.cpp:1153:5:1153:6 | * ... [post update] | PostUpdateNode should not be the target of local flow. | +| test.cpp:1153:6:1153:6 | p [inner post update] | PostUpdateNode should not be the target of local flow. | viableImplInCallContextTooLarge uniqueParameterNodeAtPosition uniqueParameterNodePosition 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 fa141b614ea6..1442f24cf154 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 @@ -327,6 +327,7 @@ irFlow | 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 | | test.cpp:1138:17:1138:22 | call to source | test.cpp:1140:8:1140:18 | * ... | +| test.cpp:1153:10:1153:15 | call to source | test.cpp:1148:10:1148:12 | * ... | | 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 b138bfb0fba5..d83af9a636d5 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -1138,4 +1138,19 @@ void test_uncertain_array(int n1, int n2) { *(data + 1) = source(); *data = 0; sink(*(data + 1)); // $ ast=1138:17 ast=1137:7 ir +} + +namespace conflation_regression { + + char* source(int); + + void read_deref_deref(char **l) { // $ ast-def=l ir-def=*l ir-def=**l + sink(**l); // $ SPURIOUS: ir + } + + void f(char ** p) // $ ast-def=p ir-def=*p ir-def=**p + { + *p = source(0); + read_deref_deref(p); + } } \ No newline at end of file From 2448475141bae32f8f393efbd5dea890c5cfb26b Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 17 Jan 2025 21:04:49 +0000 Subject: [PATCH 2/4] C++: Ensure that 'argumentOf' does not map to multiple argument positions. --- .../semmle/code/cpp/ir/dataflow/internal/DataFlowPrivate.qll | 2 +- .../lib/semmle/code/cpp/ir/dataflow/internal/DataFlowUtil.qll | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) 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 7b89e9714ff0..40740d956dcd 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 @@ -382,7 +382,7 @@ private class SideEffectArgumentNode extends ArgumentNode, SideEffectOperandNode exists(int indirectionIndex | pos = TIndirectionPosition(argumentIndex, pragma[only_bind_into](indirectionIndex)) and this.getCallInstruction() = dfCall.asCallInstruction() and - super.hasAddressOperandAndIndirectionIndex(_, pragma[only_bind_into](indirectionIndex)) + super.hasAddressOperandAndIndirectionIndex(arg, pragma[only_bind_into](indirectionIndex)) ) } } 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 4dabd917b3d3..34401de1b127 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 @@ -757,9 +757,11 @@ class SsaIteratorNode extends Node, TSsaIteratorNode { class SideEffectOperandNode extends Node instanceof IndirectOperand { CallInstruction call; int argumentIndex; + ArgumentOperand arg; SideEffectOperandNode() { - IndirectOperand.super.hasOperandAndIndirectionIndex(call.getArgumentOperand(argumentIndex), _) + arg = call.getArgumentOperand(argumentIndex) and + IndirectOperand.super.hasOperandAndIndirectionIndex(arg, _) } CallInstruction getCallInstruction() { result = call } From 8de7d4e8efc431516b491ac168d908fd929ef4c3 Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Fri, 17 Jan 2025 21:08:09 +0000 Subject: [PATCH 3/4] 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, 1 insertion(+), 2 deletions(-) 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 1442f24cf154..fa141b614ea6 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 @@ -327,7 +327,6 @@ irFlow | 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 | | test.cpp:1138:17:1138:22 | call to source | test.cpp:1140:8:1140:18 | * ... | -| test.cpp:1153:10:1153:15 | call to source | test.cpp:1148:10:1148:12 | * ... | | 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 d83af9a636d5..a65659191fb8 100644 --- a/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp +++ b/cpp/ql/test/library-tests/dataflow/dataflow-tests/test.cpp @@ -1145,7 +1145,7 @@ namespace conflation_regression { char* source(int); void read_deref_deref(char **l) { // $ ast-def=l ir-def=*l ir-def=**l - sink(**l); // $ SPURIOUS: ir + sink(**l); // Clean. Only *l is tainted } void f(char ** p) // $ ast-def=p ir-def=*p ir-def=**p From d661158feddef4061d99837a5673e893f0056bbd Mon Sep 17 00:00:00 2001 From: Mathias Vorreiter Pedersen Date: Sat, 18 Jan 2025 18:04:40 +0000 Subject: [PATCH 4/4] C++: Accept query test changes. --- .../DecompressionBombs/DecompressionBombs.expected | 9 --------- .../CWE/CWE-119/semmle/tests/UnboundedWrite.expected | 8 -------- 2 files changed, 17 deletions(-) diff --git a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409/DecompressionBombs/DecompressionBombs.expected b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409/DecompressionBombs/DecompressionBombs.expected index b372493c5baf..b813f8532cb7 100644 --- a/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409/DecompressionBombs/DecompressionBombs.expected +++ b/cpp/ql/test/experimental/query-tests/Security/CWE/CWE-409/DecompressionBombs/DecompressionBombs.expected @@ -33,13 +33,9 @@ edges | main.cpp:10:24:10:27 | minizip_test output argument | main.cpp:11:21:11:24 | **argv | provenance | | | main.cpp:10:24:10:27 | minizip_test output argument | main.cpp:12:21:12:24 | **argv | provenance | | | main.cpp:11:21:11:24 | **argv | main.cpp:11:21:11:24 | zlib_test output argument | provenance | | -| main.cpp:11:21:11:24 | **argv | main.cpp:11:21:11:24 | zlib_test output argument | provenance | | | main.cpp:11:21:11:24 | **argv | zlibTest.cpp:80:33:80:36 | **argv | provenance | | | main.cpp:11:21:11:24 | zlib_test output argument | main.cpp:12:21:12:24 | **argv | provenance | | -| main.cpp:11:21:11:24 | zlib_test output argument | main.cpp:12:21:12:24 | *argv | provenance | | | main.cpp:12:21:12:24 | **argv | zstdTest.cpp:26:39:26:42 | **argv | provenance | | -| main.cpp:12:21:12:24 | *argv | zstdTest.cpp:26:39:26:42 | **argv | provenance | | -| main.cpp:12:21:12:24 | *argv | zstdTest.cpp:26:39:26:42 | *argv | provenance | | | minizipTest.cpp:12:42:12:45 | **argv | minizipTest.cpp:12:42:12:45 | **argv | provenance | | | minizipTest.cpp:12:42:12:45 | **argv | minizipTest.cpp:17:52:17:67 | *access to array | provenance | | | minizipTest.cpp:12:42:12:45 | **argv | minizipTest.cpp:24:41:24:47 | *access to array | provenance | | @@ -107,7 +103,6 @@ edges | zlibTest.cpp:85:19:85:25 | InflateString output argument | zlibTest.cpp:80:33:80:36 | **argv | provenance | | | zlibTest.cpp:85:19:85:25 | InflateString output argument | zlibTest.cpp:80:33:80:36 | **argv [Return] | provenance | | | zstdTest.cpp:26:39:26:42 | **argv | zstdTest.cpp:27:35:27:41 | *access to array | provenance | | -| zstdTest.cpp:26:39:26:42 | *argv | zstdTest.cpp:27:35:27:41 | *access to array | provenance | | | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | provenance | | | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | zstdTest.cpp:35:52:35:54 | fin | provenance | | | zstdTest.cpp:27:35:27:41 | *access to array | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | provenance | Config | @@ -147,9 +142,7 @@ nodes | main.cpp:10:24:10:27 | minizip_test output argument | semmle.label | minizip_test output argument | | main.cpp:11:21:11:24 | **argv | semmle.label | **argv | | main.cpp:11:21:11:24 | zlib_test output argument | semmle.label | zlib_test output argument | -| main.cpp:11:21:11:24 | zlib_test output argument | semmle.label | zlib_test output argument | | main.cpp:12:21:12:24 | **argv | semmle.label | **argv | -| main.cpp:12:21:12:24 | *argv | semmle.label | *argv | | minizipTest.cpp:12:42:12:45 | **argv | semmle.label | **argv | | minizipTest.cpp:12:42:12:45 | **argv | semmle.label | **argv | | minizipTest.cpp:17:52:17:67 | *access to array | semmle.label | *access to array | @@ -199,7 +192,6 @@ nodes | zlibTest.cpp:85:19:85:25 | *access to array | semmle.label | *access to array | | zlibTest.cpp:85:19:85:25 | InflateString output argument | semmle.label | InflateString output argument | | zstdTest.cpp:26:39:26:42 | **argv | semmle.label | **argv | -| zstdTest.cpp:26:39:26:42 | *argv | semmle.label | *argv | | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | semmle.label | call to fopen_orDie | | zstdTest.cpp:27:23:27:33 | call to fopen_orDie | semmle.label | call to fopen_orDie | | zstdTest.cpp:27:35:27:41 | *access to array | semmle.label | *access to array | @@ -217,7 +209,6 @@ subpaths | main.cpp:10:24:10:27 | **argv | minizipTest.cpp:12:42:12:45 | **argv | minizipTest.cpp:12:42:12:45 | **argv | main.cpp:10:24:10:27 | minizip_test output argument | | main.cpp:11:21:11:24 | **argv | zlibTest.cpp:80:33:80:36 | **argv | zlibTest.cpp:80:33:80:36 | **argv | main.cpp:11:21:11:24 | zlib_test output argument | | main.cpp:11:21:11:24 | **argv | zlibTest.cpp:80:33:80:36 | **argv | zlibTest.cpp:80:33:80:36 | **argv [Return] | main.cpp:11:21:11:24 | zlib_test output argument | -| main.cpp:11:21:11:24 | **argv | zlibTest.cpp:80:33:80:36 | **argv | zlibTest.cpp:80:33:80:36 | **argv [Return] | main.cpp:11:21:11:24 | zlib_test output argument | | zlibTest.cpp:81:19:81:25 | *access to array | zlibTest.cpp:47:26:47:33 | *fileName | zlibTest.cpp:47:26:47:33 | *fileName | zlibTest.cpp:81:19:81:25 | UnsafeGzfread output argument | | zlibTest.cpp:82:18:82:24 | *access to array | zlibTest.cpp:57:25:57:32 | *fileName | zlibTest.cpp:57:25:57:32 | *fileName | zlibTest.cpp:82:18:82:24 | UnsafeGzgets output argument | | zlibTest.cpp:83:19:83:25 | *access to array | zlibTest.cpp:16:26:16:30 | *input | zlibTest.cpp:16:26:16:30 | *input | zlibTest.cpp:83:19:83:25 | UnsafeInflate output argument | diff --git a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/UnboundedWrite.expected b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/UnboundedWrite.expected index c71c9c7926e0..98e6fb3a0144 100644 --- a/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/UnboundedWrite.expected +++ b/cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/UnboundedWrite.expected @@ -16,8 +16,6 @@ edges | main.cpp:8:34:8:37 | **argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | provenance | | | main.cpp:8:34:8:37 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | provenance | | | main.cpp:8:34:8:37 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | provenance | | -| main.cpp:8:34:8:37 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | provenance | | -| main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | provenance | | | main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | provenance | | | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | main.cpp:9:29:9:32 | **argv | provenance | | | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | main.cpp:9:29:9:32 | *argv | provenance | | @@ -26,13 +24,10 @@ edges | main.cpp:9:29:9:32 | **argv | main.cpp:9:29:9:32 | tests_restrict_main output argument | provenance | | | main.cpp:9:29:9:32 | **argv | tests_restrict.c:15:41:15:44 | **argv | provenance | | | main.cpp:9:29:9:32 | *argv | main.cpp:9:29:9:32 | tests_restrict_main output argument | provenance | | -| main.cpp:9:29:9:32 | *argv | main.cpp:9:29:9:32 | tests_restrict_main output argument | provenance | | -| main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | **argv | provenance | | | main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | *argv | provenance | | | main.cpp:9:29:9:32 | tests_restrict_main output argument | main.cpp:10:20:10:23 | **argv | provenance | | | main.cpp:9:29:9:32 | tests_restrict_main output argument | main.cpp:10:20:10:23 | *argv | provenance | | | main.cpp:10:20:10:23 | **argv | tests.cpp:689:32:689:35 | **argv | provenance | | -| main.cpp:10:20:10:23 | *argv | tests.cpp:689:32:689:35 | **argv | provenance | | | main.cpp:10:20:10:23 | *argv | tests.cpp:689:32:689:35 | *argv | provenance | | | overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | **argv | provenance | | | overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | *argv | provenance | | @@ -98,11 +93,8 @@ subpaths | main.cpp:7:33:7:36 | **argv | overflowdestination.cpp:23:45:23:48 | **argv | overflowdestination.cpp:23:45:23:48 | *argv | main.cpp:7:33:7:36 | overflowdesination_main output argument | | main.cpp:8:34:8:37 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | | main.cpp:8:34:8:37 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | -| main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | -| main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | **argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | | main.cpp:8:34:8:37 | *argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | test_buffer_overrun.cpp:32:46:32:49 | *argv | main.cpp:8:34:8:37 | test_buffer_overrun_main output argument | | main.cpp:9:29:9:32 | **argv | tests_restrict.c:15:41:15:44 | **argv | tests_restrict.c:15:41:15:44 | **argv | main.cpp:9:29:9:32 | tests_restrict_main output argument | -| main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | **argv | tests_restrict.c:15:41:15:44 | **argv | main.cpp:9:29:9:32 | tests_restrict_main output argument | | main.cpp:9:29:9:32 | *argv | tests_restrict.c:15:41:15:44 | *argv | tests_restrict.c:15:41:15:44 | *argv | main.cpp:9:29:9:32 | tests_restrict_main output argument | #select | tests.cpp:615:2:615:7 | call to strcpy | main.cpp:6:27:6:30 | **argv | tests.cpp:615:17:615:22 | *source | This 'call to strcpy' with input from $@ may overflow the destination. | main.cpp:6:27:6:30 | **argv | a command-line argument |