Merge pull request github#14013 from alexet/only-taint-argv-indirections

CPP:Only taint argv indirections

Author: MathiasVP

cpp/ql/lib/change-notes/2023-08-24-no-taint-argv-indirections.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Only the 2 level indirection of `argv` (corresponding to `**argv`) is consided for `FlowSource`.

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

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

cpp/ql/src/change-notes/2023-08-24-no-taint-argv-indirections.md

@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* Some queries that had repeated results corresponding to different levels of indirection for `argv` now only have a single result.

cpp/ql/src/experimental/Security/CWE/CWE-078/WordexpTainted.ql

@@ -40,7 +40,7 @@ module WordexpTaintConfig implements DataFlow::ConfigSig {
 
   predicate isSink(DataFlow::Node sink) {
     exists(FunctionCall fc | fc.getTarget() instanceof WordexpFunction |
-      fc.getArgument(0) = sink.asExpr() and
+      fc.getArgument(0) = sink.asIndirectArgument(1) and
       not isCommandSubstitutionDisabled(fc)
     )
   }

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

@@ -1,16 +1,8 @@
 edges
-| test.cpp:22:27:22:30 | argv | test.cpp:29:13:29:20 | filePath |
-| test.cpp:22:27:22:30 | argv | test.cpp:29:13:29:20 | filePath |
-| test.cpp:22:27:22:30 | argv indirection | test.cpp:29:13:29:20 | filePath |
-| test.cpp:22:27:22:30 | argv indirection | test.cpp:29:13:29:20 | filePath |
+| test.cpp:22:27:22:30 | argv indirection | test.cpp:29:13:29:20 | filePath indirection |
 nodes
-| test.cpp:22:27:22:30 | argv | semmle.label | argv |
 | test.cpp:22:27:22:30 | argv indirection | semmle.label | argv indirection |
-| test.cpp:29:13:29:20 | filePath | semmle.label | filePath |
-| test.cpp:29:13:29:20 | filePath | semmle.label | filePath |
+| test.cpp:29:13:29:20 | filePath indirection | semmle.label | filePath indirection |
 subpaths
 #select
-| 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. |
-| 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. |
-| test.cpp:29:13:29:20 | filePath | test.cpp:22:27:22:30 | argv indirection | 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 | filePath | test.cpp:22:27:22:30 | argv indirection | 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 | filePath indirection | test.cpp:22:27:22:30 | argv indirection | test.cpp:29:13:29:20 | filePath indirection | 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,16 +1,10 @@
 edges
-| test.c:8:27:8:30 | argv | test.c:17:11:17:18 | fileName indirection |
 | test.c:8:27:8:30 | argv indirection | test.c:17:11:17:18 | fileName indirection |
-| test.c:8:27:8:30 | argv indirection | test.c:17:11:17:18 | fileName indirection |
-| test.c:8:27:8:30 | argv indirection | test.c:32:11:32:18 | fileName indirection |
 | test.c:8:27:8:30 | argv indirection | test.c:32:11:32:18 | fileName indirection |
 | test.c:8:27:8:30 | argv indirection | test.c:57:10:57:16 | access to array indirection |
-| test.c:8:27:8:30 | argv indirection | test.c:57:10:57:16 | access to array 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:8:27:8:30 | argv | semmle.label | argv |
-| test.c:8:27:8:30 | argv indirection | semmle.label | argv indirection |
 | test.c:8:27:8:30 | argv indirection | semmle.label | argv indirection |
 | test.c:17:11:17:18 | fileName indirection | semmle.label | fileName indirection |
 | test.c:32:11:32:18 | fileName indirection | semmle.label | fileName indirection |
@@ -21,12 +15,8 @@ nodes
 | test.c:57:10:57:16 | access to array indirection | semmle.label | access to array indirection |
 subpaths
 #select
-| 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:17:11:17:18 | fileName | test.c:8:27:8:30 | argv indirection | 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 indirection | user input (a command-line argument) |
-| test.c:17:11:17:18 | fileName | test.c:8:27:8:30 | argv indirection | 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 indirection | user input (a command-line argument) |
-| test.c:32:11:32:18 | fileName | test.c:8:27:8:30 | argv indirection | 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 indirection | user input (a command-line argument) |
 | test.c:32:11:32:18 | fileName | test.c:8:27:8:30 | argv indirection | 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 indirection | 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) |
 | test.c:57:10:57:16 | access to array | test.c:8:27:8:30 | argv indirection | test.c:57:10:57:16 | access to array indirection | This argument to a file access function is derived from $@ and then passed to read(fileName), which calls fopen(filename). | test.c:8:27:8:30 | argv indirection | user input (a command-line argument) |
-| test.c:57:10:57:16 | access to array | test.c:8:27:8:30 | argv indirection | test.c:57:10:57:16 | access to array indirection | This argument to a file access function is derived from $@ and then passed to read(fileName), which calls fopen(filename). | test.c:8:27:8:30 | argv indirection | user input (a command-line argument) |

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

@@ -1,6 +1,5 @@
 edges
 | test.cpp:15:27:15:30 | argv indirection | test.cpp:22:45:22:52 | userName indirection |
-| test.cpp:15:27:15:30 | argv indirection | 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 indirection | test.cpp:50:35:50:43 | envCflags indirection |
@@ -71,7 +70,6 @@ edges
 | test.cpp:220:19:220:26 | filename indirection | test.cpp:220:19:220:26 | filename indirection |
 nodes
 | test.cpp:15:27:15:30 | argv indirection | semmle.label | argv indirection |
-| test.cpp:15:27:15:30 | argv indirection | semmle.label | argv indirection |
 | 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 |
@@ -154,7 +152,6 @@ subpaths
 | test.cpp:196:26:196:33 | filename indirection | test.cpp:186:47:186:54 | filename indirection | test.cpp:188:11:188:17 | strncat output argument | test.cpp:196:10:196:16 | concat output argument |
 #select
 | test.cpp:23:12:23:19 | command1 | test.cpp:15:27:15:30 | argv indirection | 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 indirection | 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 indirection | 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 indirection | 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 indirection | 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 indirection | 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-089/SqlTainted/SqlTainted.expected

@@ -1,27 +1,12 @@
 edges
-| test.c:14:27:14:30 | argv | test.c:21:18:21:23 | query1 indirection |
 | test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection |
-| test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection |
-| test.cpp:39:27:39:30 | argv | test.cpp:43:27:43:33 | access to array |
-| test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array |
-| test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection |
 | test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection |
 nodes
-| test.c:14:27:14:30 | argv | semmle.label | argv |
-| test.c:14:27:14:30 | argv indirection | semmle.label | argv indirection |
 | test.c:14:27:14:30 | argv indirection | semmle.label | argv indirection |
 | test.c:21:18:21:23 | query1 indirection | semmle.label | query1 indirection |
-| test.cpp:39:27:39:30 | argv | semmle.label | argv |
 | test.cpp:39:27:39:30 | argv indirection | semmle.label | argv indirection |
-| test.cpp:39:27:39:30 | argv indirection | semmle.label | argv indirection |
-| test.cpp:43:27:43:33 | access to array | semmle.label | access to array |
 | test.cpp:43:27:43:33 | access to array indirection | semmle.label | access to array indirection |
 subpaths
 #select
-| test.c:21:18:21:23 | query1 | test.c:14:27:14:30 | argv | test.c:21:18:21:23 | query1 indirection | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | argv | user input (a command-line argument) |
-| test.c:21:18:21:23 | query1 | test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | argv indirection | user input (a command-line argument) |
 | test.c:21:18:21:23 | query1 | test.c:14:27:14:30 | argv indirection | test.c:21:18:21:23 | query1 indirection | This argument to a SQL query function is derived from $@ and then passed to mysql_query(sqlArg). | test.c:14:27:14:30 | argv indirection | user input (a command-line argument) |
-| test.cpp:43:27:43:33 | access to array | test.cpp:39:27:39:30 | argv | test.cpp:43:27:43:33 | access to array | This argument to a SQL query function is derived from $@ and then passed to pqxx::work::exec1((unnamed parameter 0)). | test.cpp:39:27:39:30 | argv | user input (a command-line argument) |
-| test.cpp:43:27:43:33 | access to array | test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array | This argument to a SQL query function is derived from $@ and then passed to pqxx::work::exec1((unnamed parameter 0)). | test.cpp:39:27:39:30 | argv indirection | user input (a command-line argument) |
-| test.cpp:43:27:43:33 | access to array | test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection | This argument to a SQL query function is derived from $@ and then passed to pqxx::work::exec1((unnamed parameter 0)). | test.cpp:39:27:39:30 | argv indirection | user input (a command-line argument) |
 | test.cpp:43:27:43:33 | access to array | test.cpp:39:27:39:30 | argv indirection | test.cpp:43:27:43:33 | access to array indirection | This argument to a SQL query function is derived from $@ and then passed to pqxx::work::exec1((unnamed parameter 0)). | test.cpp:39:27:39:30 | argv indirection | user input (a command-line argument) |

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

@@ -1,12 +1,8 @@
 edges
 | main.cpp:6:27:6:30 | argv indirection | main.cpp:7:33:7:36 | argv indirection |
-| main.cpp:6:27:6:30 | argv indirection | main.cpp:7:33:7:36 | argv indirection |
-| main.cpp:7:33:7:36 | argv indirection | overflowdestination.cpp:23:45:23:48 | argv indirection |
 | main.cpp:7:33:7:36 | argv indirection | overflowdestination.cpp:23:45:23:48 | argv indirection |
 | overflowdestination.cpp:23:45:23:48 | argv indirection | overflowdestination.cpp:30:17:30:20 | arg1 indirection |
 | overflowdestination.cpp:23:45:23:48 | argv indirection | overflowdestination.cpp:30:17:30:20 | arg1 indirection |
-| overflowdestination.cpp:23:45:23:48 | argv indirection | overflowdestination.cpp:30:17:30:20 | arg1 indirection |
-| overflowdestination.cpp:23:45:23:48 | argv indirection | overflowdestination.cpp:30:17:30:20 | arg1 indirection |
 | overflowdestination.cpp:43:8:43:10 | fgets output argument | overflowdestination.cpp:46:15:46:17 | src indirection |
 | overflowdestination.cpp:50:52:50:54 | src indirection | overflowdestination.cpp:53:9:53:12 | memcpy output argument |
 | overflowdestination.cpp:50:52:50:54 | src indirection | overflowdestination.cpp:53:15:53:17 | src indirection |
@@ -24,10 +20,7 @@ edges
 | overflowdestination.cpp:76:30:76:32 | src indirection | overflowdestination.cpp:57:52:57:54 | src indirection |
 nodes
 | main.cpp:6:27:6:30 | argv indirection | semmle.label | argv indirection |
-| main.cpp:6:27:6:30 | argv indirection | semmle.label | argv indirection |
 | main.cpp:7:33:7:36 | argv indirection | semmle.label | argv indirection |
-| main.cpp:7:33:7:36 | argv indirection | semmle.label | argv indirection |
-| overflowdestination.cpp:23:45:23:48 | argv indirection | semmle.label | argv indirection |
 | overflowdestination.cpp:23:45:23:48 | argv indirection | semmle.label | argv indirection |
 | overflowdestination.cpp:30:17:30:20 | arg1 indirection | semmle.label | arg1 indirection |
 | overflowdestination.cpp:30:17:30:20 | arg1 indirection | semmle.label | arg1 indirection |
@@ -51,8 +44,6 @@ subpaths
 #select
 | overflowdestination.cpp:30:2:30:8 | call to strncpy | main.cpp:6:27:6:30 | argv indirection | overflowdestination.cpp:30:17:30:20 | arg1 indirection | 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 indirection | overflowdestination.cpp:30:17:30:20 | arg1 indirection | 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 indirection | overflowdestination.cpp:30:17:30:20 | arg1 indirection | 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 indirection | overflowdestination.cpp:30:17:30:20 | arg1 indirection | 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 | src indirection | 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 | src indirection | 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 | src indirection | To avoid overflow, this operation should be bounded by destination-buffer size, not source-buffer size. |