From cc00f0f5846e95af2d28f0999634cedc33f59b3a Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 18 May 2020 10:39:23 +0200 Subject: [PATCH 1/3] C++: Move identical declarations to shared.h file This cleans up the test results, which were confusing because functions like `sink` had multiple locations. There are some additional results now involving casts to `const char *` because previously it varied whether `sink` used `const`, and now it always does. --- .../defaulttainttracking.cpp | 16 ++-- .../DefaultTaintTracking/global.expected | 6 +- .../dataflow/DefaultTaintTracking/globals.cpp | 4 +- .../dataflow/DefaultTaintTracking/shared.h | 14 ++++ .../DefaultTaintTracking/tainted.expected | 77 +++++++------------ .../DefaultTaintTracking/test_diff.cpp | 4 +- .../DefaultTaintTracking/test_diff.expected | 14 ++-- 7 files changed, 63 insertions(+), 72 deletions(-) create mode 100644 cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/shared.h diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp index 25fbdba93c14..b4be694589f9 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/defaulttainttracking.cpp @@ -1,13 +1,13 @@ -int atoi(const char *nptr); -char *getenv(const char *name); -char *strcat(char * s1, const char * s2); +#include "shared.h" + + + + + + + -char *strdup(const char *); -char *_strdup(const char *); -char *unmodeled_function(const char *); -void sink(const char *); -void sink(int); int main(int argc, char *argv[]) { diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/global.expected b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/global.expected index 520ebcbff1f0..5dec31c14581 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/global.expected +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/global.expected @@ -1,4 +1,6 @@ -| globals.cpp:13:15:13:20 | call to getenv | globals.cpp:2:17:2:25 | sinkParam | global1 | +| globals.cpp:13:15:13:20 | call to getenv | globals.cpp:12:10:12:16 | (const char *)... | global1 | | globals.cpp:13:15:13:20 | call to getenv | globals.cpp:12:10:12:16 | global1 | global1 | -| globals.cpp:23:15:23:20 | call to getenv | globals.cpp:2:17:2:25 | sinkParam | global2 | +| globals.cpp:13:15:13:20 | call to getenv | shared.h:5:23:5:31 | sinkparam | global1 | +| globals.cpp:23:15:23:20 | call to getenv | globals.cpp:19:10:19:16 | (const char *)... | global2 | | globals.cpp:23:15:23:20 | call to getenv | globals.cpp:19:10:19:16 | global2 | global2 | +| globals.cpp:23:15:23:20 | call to getenv | shared.h:5:23:5:31 | sinkparam | global2 | diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/globals.cpp b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/globals.cpp index 54e0718ceefb..9f598a8c6152 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/globals.cpp +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/globals.cpp @@ -1,5 +1,5 @@ -char * getenv(const char *); -void sink(char *sinkParam); +#include "shared.h" + void throughLocal() { char * local = getenv("VAR"); diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/shared.h b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/shared.h new file mode 100644 index 000000000000..0c29c16239c2 --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/shared.h @@ -0,0 +1,14 @@ +// Common declarations in this test dir should go in this file. Otherwise, some +// declarations will have multiple locations, which leads to confusing test +// output. + +void sink(const char *sinkparam); +void sink(int sinkparam); + +int atoi(const char *nptr); +char *getenv(const char *name); +char *strcat(char * s1, const char * s2); + +char *strdup(const char *string); +char *_strdup(const char *string); +char *unmodeled_function(const char *const_string); diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected index 4f98b1bead0c..c4f3af2bd7fe 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected @@ -1,22 +1,18 @@ -| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:6:15:6:24 | p#0 | -| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:14 | call to _strdup | | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:29 | (const char *)... | | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:16:16:21 | call to getenv | | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:16:16:28 | (const char *)... | -| defaulttainttracking.cpp:16:16:16:21 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | -| defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:5:14:5:23 | p#0 | -| defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:16:16:16:21 | call to getenv | shared.h:5:23:5:31 | sinkparam | +| defaulttainttracking.cpp:16:16:16:21 | call to getenv | shared.h:13:27:13:32 | string | | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:8:17:13 | call to strdup | | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:8:17:28 | (const char *)... | | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:15:17:20 | call to getenv | | defaulttainttracking.cpp:17:15:17:20 | call to getenv | defaulttainttracking.cpp:17:15:17:27 | (const char *)... | -| defaulttainttracking.cpp:17:15:17:20 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | -| defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:7:26:7:35 | p#0 | +| defaulttainttracking.cpp:17:15:17:20 | call to getenv | shared.h:5:23:5:31 | sinkparam | +| defaulttainttracking.cpp:17:15:17:20 | call to getenv | shared.h:12:26:12:31 | string | | defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:18:27:18:32 | call to getenv | | defaulttainttracking.cpp:18:27:18:32 | call to getenv | defaulttainttracking.cpp:18:27:18:39 | (const char *)... | -| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:3:38:3:39 | s2 | -| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:18:27:18:32 | call to getenv | shared.h:14:38:14:49 | const_string | | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:13 | call to strcat | | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:8:22:33 | (const char *)... | | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:20:22:25 | call to getenv | @@ -24,7 +20,8 @@ | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | (const char *)... | | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | array to pointer conversion | | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | buf | -| defaulttainttracking.cpp:22:20:22:25 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | +| defaulttainttracking.cpp:22:20:22:25 | call to getenv | shared.h:5:23:5:31 | sinkparam | +| defaulttainttracking.cpp:22:20:22:25 | call to getenv | shared.h:10:38:10:39 | s2 | | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:31:40:31:53 | dotted_address | | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:32:11:32:26 | p#0 | | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:38:11:38:21 | env_pointer | @@ -35,42 +32,37 @@ | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:36:39:61 | (const char *)... | | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:50:39:61 | & ... | | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:40:10:40:10 | a | -| defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | | defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:45:20:45:29 | p#0 | | defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | | defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p | | defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p | | defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:64:10:64:15 | call to getenv | | defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:64:10:64:22 | (const char *)... | -| defaulttainttracking.cpp:64:10:64:15 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | -| defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:64:10:64:15 | call to getenv | shared.h:5:23:5:31 | sinkparam | | defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | | defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p | | defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p | | defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:66:17:66:22 | call to getenv | | defaulttainttracking.cpp:66:17:66:22 | call to getenv | defaulttainttracking.cpp:66:17:66:29 | (const char *)... | -| defaulttainttracking.cpp:66:17:66:22 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | -| defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:66:17:66:22 | call to getenv | shared.h:5:23:5:31 | sinkparam | | defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | | defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p | | defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p | | defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:67:28:67:33 | call to getenv | | defaulttainttracking.cpp:67:28:67:33 | call to getenv | defaulttainttracking.cpp:67:28:67:40 | (const char *)... | -| defaulttainttracking.cpp:67:28:67:33 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | -| defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:67:28:67:33 | call to getenv | shared.h:5:23:5:31 | sinkparam | | defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | | defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p | | defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p | | defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:68:29:68:34 | call to getenv | | defaulttainttracking.cpp:68:29:68:34 | call to getenv | defaulttainttracking.cpp:68:29:68:41 | (const char *)... | -| defaulttainttracking.cpp:68:29:68:34 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | -| defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:68:29:68:34 | call to getenv | shared.h:5:23:5:31 | sinkparam | | defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | | defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p | | defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p | | defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:69:33:69:38 | call to getenv | | defaulttainttracking.cpp:69:33:69:38 | call to getenv | defaulttainttracking.cpp:69:33:69:45 | (const char *)... | -| defaulttainttracking.cpp:69:33:69:38 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | +| defaulttainttracking.cpp:69:33:69:38 | call to getenv | shared.h:5:23:5:31 | sinkparam | | defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:45:20:45:29 | p#0 | | defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | | defaulttainttracking.cpp:72:11:72:16 | call to getenv | defaulttainttracking.cpp:72:11:72:16 | call to getenv | @@ -87,54 +79,48 @@ | defaulttainttracking.cpp:77:34:77:39 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | | defaulttainttracking.cpp:77:34:77:39 | call to getenv | defaulttainttracking.cpp:77:34:77:39 | call to getenv | | defaulttainttracking.cpp:77:34:77:39 | call to getenv | defaulttainttracking.cpp:77:34:77:46 | (const char *)... | -| defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | | defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:57:24:57:24 | p | | defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:58:14:58:14 | p | | defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:79:30:79:35 | call to getenv | | defaulttainttracking.cpp:79:30:79:35 | call to getenv | defaulttainttracking.cpp:79:30:79:42 | (const char *)... | -| defaulttainttracking.cpp:79:30:79:35 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | -| defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:79:30:79:35 | call to getenv | shared.h:5:23:5:31 | sinkparam | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:84:17:84:17 | t | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:88:8:88:16 | call to move | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:88:8:88:32 | (const char *)... | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:88:8:88:32 | (reference dereference) | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:88:18:88:23 | call to getenv | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:88:18:88:30 | (reference to) | -| defaulttainttracking.cpp:88:18:88:23 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | -| defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | +| defaulttainttracking.cpp:88:18:88:23 | call to getenv | shared.h:5:23:5:31 | sinkparam | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:91:42:91:44 | arg | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:92:12:92:14 | arg | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:96:11:96:12 | p2 | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:97:27:97:32 | call to getenv | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | (const char *)... | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | p2 | -| defaulttainttracking.cpp:97:27:97:32 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | -| globals.cpp:5:20:5:25 | call to getenv | globals.cpp:2:17:2:25 | sinkParam | +| defaulttainttracking.cpp:97:27:97:32 | call to getenv | shared.h:5:23:5:31 | sinkparam | | globals.cpp:5:20:5:25 | call to getenv | globals.cpp:5:12:5:16 | local | | globals.cpp:5:20:5:25 | call to getenv | globals.cpp:5:20:5:25 | call to getenv | +| globals.cpp:5:20:5:25 | call to getenv | globals.cpp:6:10:6:14 | (const char *)... | | globals.cpp:5:20:5:25 | call to getenv | globals.cpp:6:10:6:14 | local | +| globals.cpp:5:20:5:25 | call to getenv | shared.h:5:23:5:31 | sinkparam | | globals.cpp:13:15:13:20 | call to getenv | globals.cpp:9:8:9:14 | global1 | | globals.cpp:13:15:13:20 | call to getenv | globals.cpp:13:15:13:20 | call to getenv | | globals.cpp:23:15:23:20 | call to getenv | globals.cpp:16:15:16:21 | global2 | | globals.cpp:23:15:23:20 | call to getenv | globals.cpp:23:15:23:20 | call to getenv | -| test_diff.cpp:92:10:92:13 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:92:10:92:13 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:92:10:92:13 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:92:10:92:13 | argv | test_diff.cpp:92:10:92:13 | argv | | test_diff.cpp:92:10:92:13 | argv | test_diff.cpp:92:10:92:16 | (const char *)... | | test_diff.cpp:92:10:92:13 | argv | test_diff.cpp:92:10:92:16 | access to array | -| test_diff.cpp:94:32:94:35 | argv | defaulttainttracking.cpp:10:11:10:13 | p#0 | -| test_diff.cpp:94:32:94:35 | argv | test_diff.cpp:2:11:2:13 | p#0 | +| test_diff.cpp:94:32:94:35 | argv | shared.h:6:15:6:23 | sinkparam | | test_diff.cpp:94:32:94:35 | argv | test_diff.cpp:94:10:94:36 | reinterpret_cast... | | test_diff.cpp:94:32:94:35 | argv | test_diff.cpp:94:32:94:35 | argv | -| test_diff.cpp:96:26:96:29 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:96:26:96:29 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:96:26:96:29 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:96:26:96:29 | argv | test_diff.cpp:16:39:16:39 | a | | test_diff.cpp:96:26:96:29 | argv | test_diff.cpp:17:10:17:10 | a | | test_diff.cpp:96:26:96:29 | argv | test_diff.cpp:96:26:96:29 | argv | | test_diff.cpp:96:26:96:29 | argv | test_diff.cpp:96:26:96:32 | (const char *)... | | test_diff.cpp:96:26:96:29 | argv | test_diff.cpp:96:26:96:32 | access to array | -| test_diff.cpp:98:18:98:21 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:98:18:98:21 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:98:18:98:21 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:98:18:98:21 | argv | test_diff.cpp:16:39:16:39 | a | | test_diff.cpp:98:18:98:21 | argv | test_diff.cpp:17:10:17:10 | a | | test_diff.cpp:98:18:98:21 | argv | test_diff.cpp:98:13:98:13 | p | @@ -148,15 +134,13 @@ | test_diff.cpp:98:18:98:21 | argv | test_diff.cpp:102:26:102:30 | * ... | | test_diff.cpp:98:18:98:21 | argv | test_diff.cpp:102:27:102:27 | p | | test_diff.cpp:98:18:98:21 | argv | test_diff.cpp:102:27:102:30 | access to array | -| test_diff.cpp:104:12:104:15 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:104:12:104:15 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:10:104:20 | (const char *)... | | test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:10:104:20 | * ... | | test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:11:104:20 | (...) | | test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:12:104:15 | argv | | test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:12:104:19 | ... + ... | -| test_diff.cpp:108:10:108:13 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:108:10:108:13 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:108:10:108:13 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:108:10:108:13 | argv | test_diff.cpp:24:20:24:29 | p#0 | | test_diff.cpp:108:10:108:13 | argv | test_diff.cpp:29:24:29:24 | p | | test_diff.cpp:108:10:108:13 | argv | test_diff.cpp:30:14:30:14 | p | @@ -168,8 +152,7 @@ | test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:111:10:111:13 | argv | | test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:111:10:111:16 | (const char *)... | | test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:111:10:111:16 | access to array | -| test_diff.cpp:115:11:115:14 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:115:11:115:14 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:115:11:115:14 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:115:11:115:14 | argv | test_diff.cpp:24:20:24:29 | p#0 | | test_diff.cpp:115:11:115:14 | argv | test_diff.cpp:41:24:41:24 | p | | test_diff.cpp:115:11:115:14 | argv | test_diff.cpp:42:14:42:14 | p | @@ -184,8 +167,7 @@ | test_diff.cpp:118:26:118:29 | argv | test_diff.cpp:118:26:118:29 | argv | | test_diff.cpp:118:26:118:29 | argv | test_diff.cpp:118:26:118:32 | (const char *)... | | test_diff.cpp:118:26:118:29 | argv | test_diff.cpp:118:26:118:32 | access to array | -| test_diff.cpp:121:23:121:26 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:121:23:121:26 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:121:23:121:26 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:121:23:121:26 | argv | test_diff.cpp:60:24:60:24 | p | | test_diff.cpp:121:23:121:26 | argv | test_diff.cpp:61:34:61:34 | p | | test_diff.cpp:121:23:121:26 | argv | test_diff.cpp:67:24:67:24 | p | @@ -193,8 +175,7 @@ | test_diff.cpp:121:23:121:26 | argv | test_diff.cpp:121:23:121:26 | argv | | test_diff.cpp:121:23:121:26 | argv | test_diff.cpp:121:23:121:29 | (const char *)... | | test_diff.cpp:121:23:121:26 | argv | test_diff.cpp:121:23:121:29 | access to array | -| test_diff.cpp:124:19:124:22 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:124:19:124:22 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:24:20:24:29 | p#0 | | test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:76:24:76:24 | p | | test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:81:24:81:24 | p | @@ -202,16 +183,14 @@ | test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:124:19:124:22 | argv | | test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:124:19:124:25 | (const char *)... | | test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:124:19:124:25 | access to array | -| test_diff.cpp:126:43:126:46 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:126:43:126:46 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:76:24:76:24 | p | | test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:81:24:81:24 | p | | test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:82:14:82:14 | p | | test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:126:43:126:46 | argv | | test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:126:43:126:49 | (const char *)... | | test_diff.cpp:126:43:126:46 | argv | test_diff.cpp:126:43:126:49 | access to array | -| test_diff.cpp:128:44:128:47 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | -| test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:1:11:1:20 | p#0 | +| test_diff.cpp:128:44:128:47 | argv | shared.h:5:23:5:31 | sinkparam | | test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:76:24:76:24 | p | | test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:81:24:81:24 | p | | test_diff.cpp:128:44:128:47 | argv | test_diff.cpp:82:14:82:14 | p | diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.cpp b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.cpp index 0de1519c49af..49306f11e366 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.cpp +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.cpp @@ -1,5 +1,5 @@ -void sink(const char *); -void sink(int); +#include "shared.h" + struct S { void(*f)(const char*); diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected index 858965a069b4..2af4a317c4a6 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected @@ -1,34 +1,30 @@ -| defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | IR only | | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:14 | call to _strdup | IR only | | defaulttainttracking.cpp:16:16:16:21 | call to getenv | defaulttainttracking.cpp:16:8:16:29 | (const char *)... | IR only | -| defaulttainttracking.cpp:16:16:16:21 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | IR only | -| defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:3:21:3:22 | s1 | AST only | +| defaulttainttracking.cpp:16:16:16:21 | call to getenv | shared.h:5:23:5:31 | sinkparam | IR only | | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:21:8:21:10 | buf | AST only | | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:22:15:22:17 | buf | AST only | | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | (const char *)... | IR only | | defaulttainttracking.cpp:22:20:22:25 | call to getenv | defaulttainttracking.cpp:24:8:24:10 | array to pointer conversion | IR only | +| defaulttainttracking.cpp:22:20:22:25 | call to getenv | shared.h:10:21:10:22 | s1 | AST only | | defaulttainttracking.cpp:38:25:38:30 | call to getenv | defaulttainttracking.cpp:39:51:39:61 | env_pointer | AST only | | defaulttainttracking.cpp:64:10:64:15 | call to getenv | defaulttainttracking.cpp:52:24:52:24 | p | IR only | -| defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | IR only | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:88:8:88:16 | call to move | IR only | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:88:8:88:32 | (const char *)... | IR only | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:88:8:88:32 | (reference dereference) | IR only | | defaulttainttracking.cpp:88:18:88:23 | call to getenv | defaulttainttracking.cpp:88:18:88:30 | (reference to) | IR only | -| defaulttainttracking.cpp:88:18:88:23 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | IR only | -| defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:9:11:9:20 | p#0 | IR only | +| defaulttainttracking.cpp:88:18:88:23 | call to getenv | shared.h:5:23:5:31 | sinkparam | IR only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:91:31:91:33 | ret | AST only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:92:5:92:8 | * ... | AST only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:92:6:92:8 | ret | AST only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:96:11:96:12 | p2 | IR only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | (const char *)... | IR only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | p2 | IR only | -| defaulttainttracking.cpp:97:27:97:32 | call to getenv | test_diff.cpp:1:11:1:20 | p#0 | IR only | +| defaulttainttracking.cpp:97:27:97:32 | call to getenv | shared.h:5:23:5:31 | sinkparam | IR only | | globals.cpp:13:15:13:20 | call to getenv | globals.cpp:13:5:13:11 | global1 | AST only | | globals.cpp:23:15:23:20 | call to getenv | globals.cpp:23:5:23:11 | global2 | AST only | | test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:11:104:20 | (...) | IR only | | test_diff.cpp:108:10:108:13 | argv | test_diff.cpp:36:24:36:24 | p | AST only | -| test_diff.cpp:111:10:111:13 | argv | defaulttainttracking.cpp:9:11:9:20 | p#0 | AST only | -| test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:1:11:1:20 | p#0 | AST only | +| test_diff.cpp:111:10:111:13 | argv | shared.h:5:23:5:31 | sinkparam | AST only | | test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:29:24:29:24 | p | AST only | | test_diff.cpp:111:10:111:13 | argv | test_diff.cpp:30:14:30:14 | p | AST only | | test_diff.cpp:124:19:124:22 | argv | test_diff.cpp:76:24:76:24 | p | IR only | From f2402c5abbc4c4af6e372f21bf7d48155223181d Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 18 May 2020 15:37:22 +0200 Subject: [PATCH 2/3] C++: Test virtual dispatch field conflation This test demonstrates that IR data flow conflates unrelated fields of a global struct-typed variable and that this bug is not present in the old AST-based implementation of `semmle.code.cpp.security.TaintTracking`. --- .../DefaultTaintTracking/dispatch.cpp | 35 +++++++++++++++++++ .../DefaultTaintTracking/tainted.expected | 35 +++++++++++++++++++ .../DefaultTaintTracking/test_diff.expected | 6 ++++ 3 files changed, 76 insertions(+) create mode 100644 cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/dispatch.cpp diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/dispatch.cpp b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/dispatch.cpp new file mode 100644 index 000000000000..7dac68a0853a --- /dev/null +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/dispatch.cpp @@ -0,0 +1,35 @@ +#include "shared.h" + +using SinkFunction = void (*)(int); + +void notSink(int notSinkParam); + +void callsSink(int sinkParam) { + sink(sinkParam); +} + +struct { + SinkFunction sinkPtr, notSinkPtr; +} globalStruct; + +union { + SinkFunction sinkPtr, notSinkPtr; +} globalUnion; + +SinkFunction globalSinkPtr; + +void assignGlobals() { + globalStruct.sinkPtr = callsSink; + globalUnion.sinkPtr = callsSink; + globalSinkPtr = callsSink; +}; + +void testStruct() { + globalStruct.sinkPtr(atoi(getenv("TAINTED"))); // should reach sinkParam [NOT DETECTED in AST] + globalStruct.notSinkPtr(atoi(getenv("TAINTED"))); // shouldn't reach sinkParam [FALSE POSITIVE in IR] + + globalUnion.sinkPtr(atoi(getenv("TAINTED"))); // should reach sinkParam + globalUnion.notSinkPtr(atoi(getenv("TAINTED"))); // should reach sinkParam + + globalSinkPtr(atoi(getenv("TAINTED"))); // should reach sinkParam +} diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected index c4f3af2bd7fe..257d9fb18847 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected @@ -98,6 +98,41 @@ | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | (const char *)... | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | p2 | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | shared.h:5:23:5:31 | sinkparam | +| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | +| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | +| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:28:24:28:27 | call to atoi | +| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:28:29:28:34 | call to getenv | +| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:28:29:28:45 | (const char *)... | +| dispatch.cpp:28:29:28:34 | call to getenv | shared.h:6:15:6:23 | sinkparam | +| dispatch.cpp:28:29:28:34 | call to getenv | shared.h:8:22:8:25 | nptr | +| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | +| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | +| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:29:27:29:30 | call to atoi | +| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:29:32:29:37 | call to getenv | +| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:29:32:29:48 | (const char *)... | +| dispatch.cpp:29:32:29:37 | call to getenv | shared.h:6:15:6:23 | sinkparam | +| dispatch.cpp:29:32:29:37 | call to getenv | shared.h:8:22:8:25 | nptr | +| dispatch.cpp:31:28:31:33 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | +| dispatch.cpp:31:28:31:33 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | +| dispatch.cpp:31:28:31:33 | call to getenv | dispatch.cpp:31:23:31:26 | call to atoi | +| dispatch.cpp:31:28:31:33 | call to getenv | dispatch.cpp:31:28:31:33 | call to getenv | +| dispatch.cpp:31:28:31:33 | call to getenv | dispatch.cpp:31:28:31:44 | (const char *)... | +| dispatch.cpp:31:28:31:33 | call to getenv | shared.h:6:15:6:23 | sinkparam | +| dispatch.cpp:31:28:31:33 | call to getenv | shared.h:8:22:8:25 | nptr | +| dispatch.cpp:32:31:32:36 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | +| dispatch.cpp:32:31:32:36 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | +| dispatch.cpp:32:31:32:36 | call to getenv | dispatch.cpp:32:26:32:29 | call to atoi | +| dispatch.cpp:32:31:32:36 | call to getenv | dispatch.cpp:32:31:32:36 | call to getenv | +| dispatch.cpp:32:31:32:36 | call to getenv | dispatch.cpp:32:31:32:47 | (const char *)... | +| dispatch.cpp:32:31:32:36 | call to getenv | shared.h:6:15:6:23 | sinkparam | +| dispatch.cpp:32:31:32:36 | call to getenv | shared.h:8:22:8:25 | nptr | +| dispatch.cpp:34:22:34:27 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | +| dispatch.cpp:34:22:34:27 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | +| dispatch.cpp:34:22:34:27 | call to getenv | dispatch.cpp:34:17:34:20 | call to atoi | +| dispatch.cpp:34:22:34:27 | call to getenv | dispatch.cpp:34:22:34:27 | call to getenv | +| dispatch.cpp:34:22:34:27 | call to getenv | dispatch.cpp:34:22:34:38 | (const char *)... | +| dispatch.cpp:34:22:34:27 | call to getenv | shared.h:6:15:6:23 | sinkparam | +| dispatch.cpp:34:22:34:27 | call to getenv | shared.h:8:22:8:25 | nptr | | globals.cpp:5:20:5:25 | call to getenv | globals.cpp:5:12:5:16 | local | | globals.cpp:5:20:5:25 | call to getenv | globals.cpp:5:20:5:25 | call to getenv | | globals.cpp:5:20:5:25 | call to getenv | globals.cpp:6:10:6:14 | (const char *)... | diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected index 2af4a317c4a6..b7ecf26c5f7e 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected @@ -20,6 +20,12 @@ | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | (const char *)... | IR only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | p2 | IR only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | shared.h:5:23:5:31 | sinkparam | IR only | +| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | IR only | +| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | IR only | +| dispatch.cpp:28:29:28:34 | call to getenv | shared.h:6:15:6:23 | sinkparam | IR only | +| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | IR only | +| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | IR only | +| dispatch.cpp:29:32:29:37 | call to getenv | shared.h:6:15:6:23 | sinkparam | IR only | | globals.cpp:13:15:13:20 | call to getenv | globals.cpp:13:5:13:11 | global1 | AST only | | globals.cpp:23:15:23:20 | call to getenv | globals.cpp:23:5:23:11 | global2 | AST only | | test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:11:104:20 | (...) | IR only | From 76e194c8bea582e737bd4abf9a07140f8fb72fb2 Mon Sep 17 00:00:00 2001 From: Jonas Jensen Date: Mon, 18 May 2020 16:19:16 +0200 Subject: [PATCH 3/3] C++: Fix struct field conflation in IR data flow The virtual-dispatch code for globals was missing any relationship between the union field access and the global variable, which meant it propagated function-pointer flow between any two fields of a global struct. This resulted in false positives from `cpp/tainted-format-string` on projects using SDL, such as WohlSoft/PGE-Project. In addition to fixing that bug, this commit also brings the code up to date with the new style of modeling flow through global variables: `DataFlow::Node.asVariable()`. --- .../ir/dataflow/internal/DataFlowDispatch.qll | 79 +++++++++---------- .../DefaultTaintTracking/dispatch.cpp | 4 +- .../DefaultTaintTracking/tainted.expected | 6 -- .../DefaultTaintTracking/test_diff.expected | 6 -- 4 files changed, 41 insertions(+), 54 deletions(-) diff --git a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowDispatch.qll b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowDispatch.qll index 1dd1d9ac4cc1..9ee685630dd7 100644 --- a/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowDispatch.qll +++ b/cpp/ql/src/semmle/code/cpp/ir/dataflow/internal/DataFlowDispatch.qll @@ -77,49 +77,48 @@ private module VirtualDispatch { // Local flow DataFlow::localFlowStep(src, other) and allowFromArg = allowOtherFromArg - ) - or - // Flow through global variable - exists(StoreInstruction store | - store = src.asInstruction() and - ( - exists(Variable var | - var = store.getDestinationAddress().(VariableAddressInstruction).getASTVariable() and - this.flowsFromGlobal(var) + or + // Flow from global variable to load. + exists(LoadInstruction load, GlobalOrNamespaceVariable var | + var = src.asVariable() and + other.asInstruction() = load and + // The `allowFromArg` concept doesn't play a role when `src` is a + // global variable, so we just set it to a single arbitrary value for + // performance. + allowFromArg = true + | + // Load directly from the global variable + load.getSourceAddress().(VariableAddressInstruction).getASTVariable() = var + or + // Load from a field on a global union + exists(FieldAddressInstruction fa | + fa = load.getSourceAddress() and + fa.getObjectAddress().(VariableAddressInstruction).getASTVariable() = var and + fa.getField().getDeclaringType() instanceof Union ) + ) + or + // Flow from store to global variable. These cases are similar to the + // above but have `StoreInstruction` instead of `LoadInstruction` and + // have the roles swapped between `other` and `src`. + exists(StoreInstruction store, GlobalOrNamespaceVariable var | + var = other.asVariable() and + store = src.asInstruction() and + // Setting `allowFromArg` to `true` like in the base case means we + // treat a store to a global variable like the dispatch itself: flow + // may come from anywhere. + allowFromArg = true + | + // Store directly to the global variable + store.getDestinationAddress().(VariableAddressInstruction).getASTVariable() = var or - exists(Variable var, FieldAccess a | - var = - store - .getDestinationAddress() - .(FieldAddressInstruction) - .getObjectAddress() - .(VariableAddressInstruction) - .getASTVariable() and - this.flowsFromGlobalUnionField(var, a) + // Store to a field on a global union + exists(FieldAddressInstruction fa | + fa = store.getDestinationAddress() and + fa.getObjectAddress().(VariableAddressInstruction).getASTVariable() = var and + fa.getField().getDeclaringType() instanceof Union ) - ) and - allowFromArg = true - ) - } - - private predicate flowsFromGlobal(GlobalOrNamespaceVariable var) { - exists(LoadInstruction load | - this.flowsFrom(DataFlow::instructionNode(load), _) and - load.getSourceAddress().(VariableAddressInstruction).getASTVariable() = var - ) - } - - private predicate flowsFromGlobalUnionField(Variable var, FieldAccess a) { - a.getTarget().getDeclaringType() instanceof Union and - exists(LoadInstruction load | - this.flowsFrom(DataFlow::instructionNode(load), _) and - load - .getSourceAddress() - .(FieldAddressInstruction) - .getObjectAddress() - .(VariableAddressInstruction) - .getASTVariable() = var + ) ) } } diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/dispatch.cpp b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/dispatch.cpp index 7dac68a0853a..929983d05f72 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/dispatch.cpp +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/dispatch.cpp @@ -25,8 +25,8 @@ void assignGlobals() { }; void testStruct() { - globalStruct.sinkPtr(atoi(getenv("TAINTED"))); // should reach sinkParam [NOT DETECTED in AST] - globalStruct.notSinkPtr(atoi(getenv("TAINTED"))); // shouldn't reach sinkParam [FALSE POSITIVE in IR] + globalStruct.sinkPtr(atoi(getenv("TAINTED"))); // should reach sinkParam [NOT DETECTED] + globalStruct.notSinkPtr(atoi(getenv("TAINTED"))); // shouldn't reach sinkParam globalUnion.sinkPtr(atoi(getenv("TAINTED"))); // should reach sinkParam globalUnion.notSinkPtr(atoi(getenv("TAINTED"))); // should reach sinkParam diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected index 257d9fb18847..d02f89bc325d 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/tainted.expected @@ -98,19 +98,13 @@ | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | (const char *)... | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | p2 | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | shared.h:5:23:5:31 | sinkparam | -| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | -| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | | dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:28:24:28:27 | call to atoi | | dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:28:29:28:34 | call to getenv | | dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:28:29:28:45 | (const char *)... | -| dispatch.cpp:28:29:28:34 | call to getenv | shared.h:6:15:6:23 | sinkparam | | dispatch.cpp:28:29:28:34 | call to getenv | shared.h:8:22:8:25 | nptr | -| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | -| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | | dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:29:27:29:30 | call to atoi | | dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:29:32:29:37 | call to getenv | | dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:29:32:29:48 | (const char *)... | -| dispatch.cpp:29:32:29:37 | call to getenv | shared.h:6:15:6:23 | sinkparam | | dispatch.cpp:29:32:29:37 | call to getenv | shared.h:8:22:8:25 | nptr | | dispatch.cpp:31:28:31:33 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | | dispatch.cpp:31:28:31:33 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | diff --git a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected index b7ecf26c5f7e..2af4a317c4a6 100644 --- a/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected +++ b/cpp/ql/test/library-tests/dataflow/DefaultTaintTracking/test_diff.expected @@ -20,12 +20,6 @@ | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | (const char *)... | IR only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | defaulttainttracking.cpp:98:10:98:11 | p2 | IR only | | defaulttainttracking.cpp:97:27:97:32 | call to getenv | shared.h:5:23:5:31 | sinkparam | IR only | -| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | IR only | -| dispatch.cpp:28:29:28:34 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | IR only | -| dispatch.cpp:28:29:28:34 | call to getenv | shared.h:6:15:6:23 | sinkparam | IR only | -| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:7:20:7:28 | sinkParam | IR only | -| dispatch.cpp:29:32:29:37 | call to getenv | dispatch.cpp:8:8:8:16 | sinkParam | IR only | -| dispatch.cpp:29:32:29:37 | call to getenv | shared.h:6:15:6:23 | sinkparam | IR only | | globals.cpp:13:15:13:20 | call to getenv | globals.cpp:13:5:13:11 | global1 | AST only | | globals.cpp:23:15:23:20 | call to getenv | globals.cpp:23:5:23:11 | global2 | AST only | | test_diff.cpp:104:12:104:15 | argv | test_diff.cpp:104:11:104:20 | (...) | IR only |