C++: Do not use isReturnValue in getenv, gets, and fgets models

Author: jketema

cpp/ql/lib/semmle/code/cpp/models/implementations/Getenv.qll

@@ -16,10 +16,7 @@ class Getenv extends LocalFlowSourceFunction {
   }
 
   override predicate hasLocalFlowSource(FunctionOutput output, string description) {
-    (
-      output.isReturnValueDeref() or
-      output.isReturnValue()
-    ) and
+    output.isReturnValueDeref() and
     description = "an environment variable"
   }
 }

cpp/ql/lib/semmle/code/cpp/models/implementations/Gets.qll

@@ -51,7 +51,6 @@ private class FgetsFunction extends DataFlowFunction, TaintFunction, ArrayFuncti
   override predicate hasRemoteFlowSource(FunctionOutput output, string description) {
     (
       output.isParameterDeref(0) or
-      output.isReturnValue() or
       output.isReturnValueDeref()
     ) and
     description = "string read by " + this.getName()
@@ -102,7 +101,6 @@ private class GetsFunction extends DataFlowFunction, ArrayFunction, AliasFunctio
   override predicate hasLocalFlowSource(FunctionOutput output, string description) {
     (
       output.isParameterDeref(0) or
-      output.isReturnValue() or
       output.isReturnValueDeref()
     ) and
     description = "string read by " + this.getName()

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

@@ -14,18 +14,15 @@ edges
 | test.cpp:91:9:91:16 | fread output argument | test.cpp:93:17:93:24 | filename indirection |
 | test.cpp:93:11:93:14 | strncat output argument | test.cpp:94:45:94:48 | path indirection |
 | test.cpp:93:17:93:24 | filename indirection | test.cpp:93:11:93:14 | strncat output argument |
-| test.cpp:106:20:106:38 | call to getenv | test.cpp:107:33:107:36 | path indirection |
 | test.cpp:106:20:106:38 | call to getenv indirection | test.cpp:107:33:107:36 | path indirection |
 | test.cpp:107:31:107:31 | call to operator+ | test.cpp:108:18:108:22 | call to c_str indirection |
 | test.cpp:107:33:107:36 | path indirection | test.cpp:107:31:107:31 | call to operator+ |
-| test.cpp:113:20:113:38 | call to getenv | test.cpp:114:19:114:22 | path indirection |
 | test.cpp:113:20:113:38 | call to getenv indirection | test.cpp:114:19:114:22 | path indirection |
 | test.cpp:114:10:114:23 | call to operator+ | test.cpp:114:25:114:29 | call to c_str indirection |
 | test.cpp:114:10:114:23 | call to operator+ | test.cpp:114:25:114:29 | call to c_str indirection |
 | test.cpp:114:17:114:17 | call to operator+ | test.cpp:114:10:114:23 | call to operator+ |
 | test.cpp:114:19:114:22 | path indirection | test.cpp:114:10:114:23 | call to operator+ |
 | test.cpp:114:19:114:22 | path indirection | test.cpp:114:17:114:17 | call to operator+ |
-| test.cpp:119:20:119:38 | call to getenv | test.cpp:120:19:120:22 | path indirection |
 | test.cpp:119:20:119:38 | call to getenv indirection | test.cpp:120:19:120:22 | path indirection |
 | test.cpp:120:17:120:17 | call to operator+ | test.cpp:120:10:120:30 | call to data indirection |
 | test.cpp:120:19:120:22 | path indirection | test.cpp:120:17:120:17 | call to operator+ |
@@ -89,20 +86,17 @@ nodes
 | test.cpp:93:11:93:14 | strncat output argument | semmle.label | strncat output argument |
 | test.cpp:93:17:93:24 | filename indirection | semmle.label | filename indirection |
 | test.cpp:94:45:94:48 | path indirection | semmle.label | path indirection |
-| test.cpp:106:20:106:38 | call to getenv | semmle.label | call to getenv |
 | test.cpp:106:20:106:38 | call to getenv indirection | semmle.label | call to getenv indirection |
 | test.cpp:107:31:107:31 | call to operator+ | semmle.label | call to operator+ |
 | test.cpp:107:33:107:36 | path indirection | semmle.label | path indirection |
 | test.cpp:108:18:108:22 | call to c_str indirection | semmle.label | call to c_str indirection |
-| test.cpp:113:20:113:38 | call to getenv | semmle.label | call to getenv |
 | test.cpp:113:20:113:38 | call to getenv indirection | semmle.label | call to getenv indirection |
 | test.cpp:114:10:114:23 | call to operator+ | semmle.label | call to operator+ |
 | test.cpp:114:10:114:23 | call to operator+ | semmle.label | call to operator+ |
 | test.cpp:114:17:114:17 | call to operator+ | semmle.label | call to operator+ |
 | test.cpp:114:19:114:22 | path indirection | semmle.label | path indirection |
 | test.cpp:114:25:114:29 | call to c_str indirection | semmle.label | call to c_str indirection |
 | test.cpp:114:25:114:29 | call to c_str indirection | semmle.label | call to c_str indirection |
-| test.cpp:119:20:119:38 | call to getenv | semmle.label | call to getenv |
 | test.cpp:119:20:119:38 | call to getenv indirection | semmle.label | call to getenv indirection |
 | test.cpp:120:10:120:30 | call to data indirection | semmle.label | call to data indirection |
 | test.cpp:120:17:120:17 | call to operator+ | semmle.label | call to operator+ |
@@ -156,13 +150,9 @@ subpaths
 | 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 |
 | test.cpp:94:45:94:48 | path | test.cpp:91:9:91:16 | fread output argument | test.cpp:94:45:94:48 | path indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl. | test.cpp:91:9:91:16 | fread output argument | user input (string read by fread) | test.cpp:93:11:93:14 | strncat output argument | strncat output argument |
-| test.cpp:108:18:108:22 | call to c_str | test.cpp:106:20:106:38 | call to getenv | test.cpp:108:18:108:22 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:106:20:106:38 | call to getenv | user input (an environment variable) | test.cpp:107:31:107:31 | call to operator+ | call to operator+ |
 | test.cpp:108:18:108:22 | call to c_str | test.cpp:106:20:106:38 | call to getenv indirection | test.cpp:108:18:108:22 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:106:20:106:38 | call to getenv indirection | user input (an environment variable) | test.cpp:107:31:107:31 | call to operator+ | call to operator+ |
-| test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:38 | call to getenv | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:38 | call to getenv | user input (an environment variable) | test.cpp:114:10:114:23 | call to operator+ | call to operator+ |
-| test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:38 | call to getenv | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:38 | call to getenv | user input (an environment variable) | test.cpp:114:17:114:17 | call to operator+ | call to operator+ |
 | test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:38 | call to getenv indirection | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:38 | call to getenv indirection | user input (an environment variable) | test.cpp:114:10:114:23 | call to operator+ | call to operator+ |
 | test.cpp:114:25:114:29 | call to c_str | test.cpp:113:20:113:38 | call to getenv indirection | test.cpp:114:25:114:29 | call to c_str indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:113:20:113:38 | call to getenv indirection | user input (an environment variable) | test.cpp:114:17:114:17 | call to operator+ | call to operator+ |
-| test.cpp:120:25:120:28 | call to data | test.cpp:119:20:119:38 | call to getenv | test.cpp:120:10:120:30 | call to data indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:119:20:119:38 | call to getenv | user input (an environment variable) | test.cpp:120:17:120:17 | call to operator+ | call to operator+ |
 | test.cpp:120:25:120:28 | call to data | test.cpp:119:20:119:38 | call to getenv indirection | test.cpp:120:10:120:30 | call to data indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:119:20:119:38 | call to getenv indirection | user input (an environment variable) | test.cpp:120:17:120:17 | call to operator+ | call to operator+ |
 | test.cpp:143:10:143:16 | command | test.cpp:140:9:140:11 | fread output argument | test.cpp:143:10:143:16 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to system(string). | test.cpp:140:9:140:11 | fread output argument | user input (string read by fread) | test.cpp:142:11:142:17 | sprintf output argument | sprintf output argument |
 | test.cpp:183:32:183:38 | command | test.cpp:174:9:174:16 | fread output argument | test.cpp:183:32:183:38 | command indirection | This argument to an OS command is derived from $@, dangerously concatenated into $@, and then passed to execl. | test.cpp:174:9:174:16 | fread output argument | user input (string read by fread) | test.cpp:177:13:177:17 | strncat output argument | strncat output argument |