PS: Add tests showing that there is no flow starting at environment variables, but we still have flow through them.

Author: MathiasVP

powershell/ql/lib/semmle/code/powershell/security/CommandInjectionCustomizations.qll

@@ -31,7 +31,12 @@ module CommandInjection {
   abstract class Sanitizer extends DataFlow::Node { }
 
   /** A source of user input, considered as a flow source for command injection. */
-  class FlowSourceAsSource extends Source instanceof SourceNode {
+  class FlowSourceAsSource extends Source {
+    FlowSourceAsSource() {
+      this instanceof SourceNode and
+      not this instanceof EnvironmentVariableSource
+    }
+
     override string getSourceType() { result = "user-provided value" }
   }
 

powershell/ql/test/query-tests/security/cwe-078/CommandInjection/CommandInjection.expected

@@ -54,6 +54,7 @@ edges
 | test.ps1:172:42:172:47 | input | test.ps1:136:11:136:20 | userinput | provenance |  |
 | test.ps1:173:42:173:47 | input | test.ps1:144:11:144:20 | userinput | provenance |  |
 | test.ps1:214:10:214:32 | Call to read-host | test.ps1:217:7:217:10 | $o | provenance | Src:MaD:0  |
+| test.ps1:225:14:225:36 | Call to read-host | test.ps1:229:7:229:10 | $y | provenance | Src:MaD:0  |
 nodes
 | test.ps1:3:11:3:20 | userinput | semmle.label | userinput |
 | test.ps1:4:23:4:52 | Get-Process -Name $UserInput | semmle.label | Get-Process -Name $UserInput |
@@ -112,6 +113,8 @@ nodes
 | test.ps1:173:42:173:47 | input | semmle.label | input |
 | test.ps1:214:10:214:32 | Call to read-host | semmle.label | Call to read-host |
 | test.ps1:217:7:217:10 | $o | semmle.label | $o |
+| test.ps1:225:14:225:36 | Call to read-host | semmle.label | Call to read-host |
+| test.ps1:229:7:229:10 | $y | semmle.label | $y |
 subpaths
 #select
 | test.ps1:4:23:4:52 | Get-Process -Name $UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:4:23:4:52 | Get-Process -Name $UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value |
@@ -133,3 +136,4 @@ subpaths
 | test.ps1:139:50:139:59 | UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:139:50:139:59 | UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value |
 | test.ps1:147:63:147:72 | UserInput | test.ps1:152:10:152:32 | Call to read-host | test.ps1:147:63:147:72 | UserInput | This command depends on a $@. | test.ps1:152:10:152:32 | Call to read-host | user-provided value |
 | test.ps1:217:7:217:10 | $o | test.ps1:214:10:214:32 | Call to read-host | test.ps1:217:7:217:10 | $o | This command depends on a $@. | test.ps1:214:10:214:32 | Call to read-host | user-provided value |
+| test.ps1:229:7:229:10 | $y | test.ps1:225:14:225:36 | Call to read-host | test.ps1:229:7:229:10 | $y | This command depends on a $@. | test.ps1:225:14:225:36 | Call to read-host | user-provided value |

powershell/ql/test/query-tests/security/cwe-078/CommandInjection/test.ps1

@@ -215,4 +215,16 @@ function false-positive-in-call-operator($d)
     & unzip -o "$o" -d $d # GOOD
 
     . "$o" # BAD
+}
+
+function flow-through-env-var() {
+    $x = $env:foo
+
+    . "$x" # GOOD # we don't consider environment vars flow sources
+
+    $input = Read-Host "enter input"
+    $env:bar = $input
+
+    $y = $env:bar
+    . "$y" # BAD # but we have flow through them
 }