Merge pull request #11693 from jketema/argv-param-flowsource

C++: Define the `argv` flow source in terms the input parameter

Author: MathiasVP

cpp/ql/lib/change-notes/2022-12-19-argv-as-parameter-flowsource.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `ArgvSource` flow source now uses the second parameter of `main` as its source instead of the uses of this parameter.

cpp/ql/lib/semmle/code/cpp/security/FlowSources.qll

@@ -92,7 +92,7 @@ private class ArgvSource extends LocalFlowSource {
     exists(Function main, Parameter argv |
       main.hasGlobalName("main") and
       main.getParameter(1) = argv and
-      this.asExpr() = argv.getAnAccess()
+      this.asParameter() = argv
     )
   }
 

cpp/ql/src/Security/CWE/CWE-022/TaintedPath.ql

@@ -91,8 +91,6 @@ class TaintedPathConfiguration extends TaintTracking::Configuration {
     )
   }
 
-  override predicate isSanitizerIn(DataFlow::Node node) { this.isSource(node) }
-
   override predicate isSanitizer(DataFlow::Node node) {
     node.asExpr().(Call).getTarget().getUnspecifiedType() instanceof ArithmeticType
     or

cpp/ql/test/experimental/query-tests/Security/CWE/CWE-078/WordexpTainted.expected

@@ -1,11 +1,11 @@
 edges
-| test.cpp:23:20:23:23 | argv | test.cpp:29:13:29:20 | (const char *)... |
-| test.cpp:23:20:23:23 | argv | test.cpp:29:13:29:20 | filePath |
+| test.cpp:22:27:22:30 | argv | test.cpp:29:13:29:20 | (const char *)... |
+| test.cpp:22:27:22:30 | argv | test.cpp:29:13:29:20 | filePath |
 nodes
-| test.cpp:23:20:23:23 | argv | semmle.label | argv |
+| test.cpp:22:27:22:30 | argv | semmle.label | argv |
 | test.cpp:29:13:29:20 | (const char *)... | semmle.label | (const char *)... |
 | test.cpp:29:13:29:20 | filePath | semmle.label | filePath |
 subpaths
 #select
-| test.cpp:29:13:29:20 | (const char *)... | test.cpp:23:20:23:23 | argv | test.cpp:29:13:29:20 | (const char *)... | Using user-supplied data in a `wordexp` command, without disabling command substitution, can make code vulnerable to command injection. |
-| test.cpp:29:13:29:20 | filePath | test.cpp:23:20:23:23 | argv | test.cpp:29:13:29:20 | filePath | Using user-supplied data in a `wordexp` command, without disabling command substitution, can make code vulnerable to command injection. |
+| test.cpp:29:13:29:20 | (const char *)... | test.cpp:22:27:22:30 | argv | test.cpp:29:13:29:20 | (const char *)... | Using user-supplied data in a `wordexp` command, without disabling command substitution, can make code vulnerable to command injection. |
+| test.cpp:29:13:29:20 | filePath | test.cpp:22:27:22:30 | argv | test.cpp:29:13:29:20 | filePath | Using user-supplied data in a `wordexp` command, without disabling command substitution, can make code vulnerable to command injection. |

cpp/ql/test/query-tests/Security/CWE/CWE-022/semmle/tests/TaintedPath.expected

@@ -1,20 +1,19 @@
 edges
-| test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName indirection |
-| test.c:31:22:31:25 | argv | test.c:32:11:32:18 | fileName indirection |
+| test.c:8:27:8:30 | argv | test.c:17:11:17:18 | fileName indirection |
+| test.c:8:27:8:30 | argv | test.c:32:11:32:18 | fileName indirection |
 | test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | fileName indirection |
 | test.c:43:17:43:24 | scanf output argument | test.c:44:11:44:18 | fileName indirection |
 nodes
-| test.c:9:23:9:26 | argv | semmle.label | argv |
+| test.c:8:27:8:30 | argv | semmle.label | argv |
 | test.c:17:11:17:18 | fileName indirection | semmle.label | fileName indirection |
-| test.c:31:22:31:25 | argv | semmle.label | argv |
 | test.c:32:11:32:18 | fileName indirection | semmle.label | fileName indirection |
 | test.c:37:17:37:24 | scanf output argument | semmle.label | scanf output argument |
 | test.c:38:11:38:18 | fileName indirection | semmle.label | fileName indirection |
 | test.c:43:17:43:24 | scanf output argument | semmle.label | scanf output argument |
 | test.c:44:11:44:18 | fileName indirection | semmle.label | fileName indirection |
 subpaths
 #select
-| test.c:17:11:17:18 | fileName | test.c:9:23:9:26 | argv | test.c:17:11:17:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:9:23:9:26 | argv | user input (a command-line argument) |
-| test.c:32:11:32:18 | fileName | test.c:31:22:31:25 | argv | test.c:32:11:32:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:31:22:31:25 | argv | user input (a command-line argument) |
+| test.c:17:11:17:18 | fileName | test.c:8:27:8:30 | argv | test.c:17:11:17:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:8:27:8:30 | argv | user input (a command-line argument) |
+| test.c:32:11:32:18 | fileName | test.c:8:27:8:30 | argv | test.c:32:11:32:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:8:27:8:30 | argv | user input (a command-line argument) |
 | test.c:38:11:38:18 | fileName | test.c:37:17:37:24 | scanf output argument | test.c:38:11:38:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:37:17:37:24 | scanf output argument | user input (value read by scanf) |
 | test.c:44:11:44:18 | fileName | test.c:43:17:43:24 | scanf output argument | test.c:44:11:44:18 | fileName indirection | This argument to a file access function is derived from $@ and then passed to fopen(filename). | test.c:43:17:43:24 | scanf output argument | user input (value read by scanf) |

cpp/ql/test/query-tests/Security/CWE/CWE-078/semmle/ExecTainted/ExecTainted.expected

@@ -1,5 +1,5 @@
 edges
-| test.cpp:16:20:16:23 | argv | test.cpp:22:45:22:52 | userName indirection |
+| test.cpp:15:27:15:30 | argv | test.cpp:22:45:22:52 | userName indirection |
 | test.cpp:22:13:22:20 | sprintf output argument | test.cpp:23:12:23:19 | command1 indirection |
 | test.cpp:22:45:22:52 | userName indirection | test.cpp:22:13:22:20 | sprintf output argument |
 | test.cpp:47:21:47:26 | call to getenv | test.cpp:50:35:50:43 | envCflags indirection |
@@ -74,7 +74,7 @@ edges
 | test.cpp:220:19:220:26 | filename indirection | test.cpp:220:10:220:16 | strncat output argument |
 | test.cpp:220:19:220:26 | filename indirection | test.cpp:220:10:220:16 | strncat output argument |
 nodes
-| test.cpp:16:20:16:23 | argv | semmle.label | argv |
+| test.cpp:15:27:15:30 | argv | semmle.label | argv |
 | test.cpp:22:13:22:20 | sprintf output argument | semmle.label | sprintf output argument |
 | test.cpp:22:45:22:52 | userName indirection | semmle.label | userName indirection |
 | test.cpp:23:12:23:19 | command1 indirection | semmle.label | command1 indirection |
@@ -161,7 +161,7 @@ subpaths
 | test.cpp:196:26:196:33 | filename indirection | test.cpp:186:47:186:54 | *filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
 | test.cpp:196:26:196:33 | filename indirection | test.cpp:186:47:186:54 | *filename | test.cpp:188:11:188:17 | command [post update] | test.cpp:196:10:196:16 | command [post update] |
 #select
-| test.cpp:23:12:23:19 | command1 | test.cpp:16:20:16:23 | argv | test.cpp:23:12:23:19 | command1 indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:16:20:16:23 | argv | user input (a command-line argument) | test.cpp:22:13:22:20 | sprintf output argument | sprintf output argument |
+| test.cpp:23:12:23:19 | command1 | test.cpp:15:27:15:30 | argv | test.cpp:23:12:23:19 | command1 indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:15:27:15:30 | argv | user input (a command-line argument) | test.cpp:22:13:22:20 | sprintf output argument | sprintf output argument |
 | test.cpp:51:10:51:16 | command | test.cpp:47:21:47:26 | call to getenv | test.cpp:51:10:51:16 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:47:21:47:26 | call to getenv | user input (an environment variable) | test.cpp:50:11:50:17 | sprintf output argument | sprintf output argument |
 | test.cpp:65:10:65:16 | command | test.cpp:62:9:62:16 | fread output argument | test.cpp:65:10:65:16 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:62:9:62:16 | fread output argument | user input (string read by fread) | test.cpp:64:11:64:17 | strncat output argument | strncat output argument |
 | test.cpp:85:32:85:38 | command | test.cpp:82:9:82:16 | fread output argument | test.cpp:85:32:85:38 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl. | test.cpp:82:9:82:16 | fread output argument | user input (string read by fread) | test.cpp:84:11:84:17 | strncat output argument | strncat output argument |

cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/OverflowDestination.expected

@@ -1,5 +1,10 @@
 edges
-| overflowdestination.cpp:27:9:27:12 | argv | overflowdestination.cpp:30:17:30:20 | (const char *)... |
+| main.cpp:6:27:6:30 | argv | main.cpp:7:33:7:36 | argv |
+| main.cpp:6:27:6:30 | argv | main.cpp:7:33:7:36 | argv indirection |
+| main.cpp:7:33:7:36 | argv | overflowdestination.cpp:23:45:23:48 | argv |
+| main.cpp:7:33:7:36 | argv indirection | overflowdestination.cpp:23:45:23:48 | *argv |
+| overflowdestination.cpp:23:45:23:48 | *argv | overflowdestination.cpp:30:17:30:20 | (const char *)... |
+| overflowdestination.cpp:23:45:23:48 | argv | overflowdestination.cpp:30:17:30:20 | (const char *)... |
 | overflowdestination.cpp:43:8:43:10 | fgets output argument | overflowdestination.cpp:46:15:46:17 | (const void *)... |
 | overflowdestination.cpp:50:52:50:54 | *src | overflowdestination.cpp:50:52:50:54 | ReturnIndirection |
 | overflowdestination.cpp:50:52:50:54 | src | overflowdestination.cpp:53:15:53:17 | (const void *)... |
@@ -17,7 +22,11 @@ edges
 | overflowdestination.cpp:76:30:76:32 | src | overflowdestination.cpp:57:52:57:54 | src |
 | overflowdestination.cpp:76:30:76:32 | src indirection | overflowdestination.cpp:57:52:57:54 | *src |
 nodes
-| overflowdestination.cpp:27:9:27:12 | argv | semmle.label | argv |
+| main.cpp:6:27:6:30 | argv | semmle.label | argv |
+| main.cpp:7:33:7:36 | argv | semmle.label | argv |
+| main.cpp:7:33:7:36 | argv indirection | semmle.label | argv indirection |
+| overflowdestination.cpp:23:45:23:48 | *argv | semmle.label | *argv |
+| overflowdestination.cpp:23:45:23:48 | argv | semmle.label | argv |
 | overflowdestination.cpp:30:17:30:20 | (const char *)... | semmle.label | (const char *)... |
 | overflowdestination.cpp:43:8:43:10 | fgets output argument | semmle.label | fgets output argument |
 | overflowdestination.cpp:46:15:46:17 | (const void *)... | semmle.label | (const void *)... |
@@ -37,7 +46,7 @@ nodes
 subpaths
 | overflowdestination.cpp:75:30:75:32 | src indirection | overflowdestination.cpp:50:52:50:54 | *src | overflowdestination.cpp:50:52:50:54 | ReturnIndirection | overflowdestination.cpp:75:30:75:32 | overflowdest_test2 output argument |
 #select
-| overflowdestination.cpp:30:2:30:8 | call to strncpy | overflowdestination.cpp:27:9:27:12 | argv | overflowdestination.cpp:30:17:30:20 | (const char *)... | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
+| overflowdestination.cpp:30:2:30:8 | call to strncpy | main.cpp:6:27:6:30 | argv | overflowdestination.cpp:30:17:30:20 | (const char *)... | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
 | overflowdestination.cpp:46:2:46:7 | call to memcpy | overflowdestination.cpp:43:8:43:10 | fgets output argument | overflowdestination.cpp:46:15:46:17 | (const void *)... | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
 | overflowdestination.cpp:53:2:53:7 | call to memcpy | overflowdestination.cpp:73:8:73:10 | fgets output argument | overflowdestination.cpp:53:15:53:17 | (const void *)... | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |
 | overflowdestination.cpp:64:2:64:7 | call to memcpy | overflowdestination.cpp:73:8:73:10 | fgets output argument | overflowdestination.cpp:64:16:64:19 | (const void *)... | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |

cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/main.cpp

@@ -0,0 +1,12 @@
+int overflowdesination_main(int argc, char **argv);
+int test_buffer_overrun_main(int argc, char **argv);
+int tests_restrict_main(int argc, char **argv);
+int tests_main(int argc, char **argv);
+
+int main(int argc, char **argv) {
+  overflowdesination_main(argc, argv);
+  test_buffer_overrun_main(argc, argv);
+  tests_restrict_main(argc, argv);
+  tests_main(argc, argv);
+  return 0;
+}

cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/overflowdestination.cpp

@@ -20,7 +20,7 @@ inline size_t min(size_t a, size_t b) {
 	}
 }
 
-int main(int argc, char* argv[]) {
+int overflowdesination_main(int argc, char* argv[]) {
 	char param[20];
 	char *arg1;
 

cpp/ql/test/query-tests/Security/CWE/CWE-119/semmle/tests/test_buffer_overrun.cpp

@@ -29,7 +29,7 @@ void test_buffer_overrun_in_while_loop_using_array_indexing()
     }
 }
 
-int main(int argc, char *argv[])
+int test_buffer_overrun_main(int argc, char *argv[])
 {
     test_buffer_overrun_in_for_loop();
     test_buffer_overrun_in_while_loop_using_pointer_arithmetic();