JS: List safe environment variables in IndirectCommandInjection

Author: asgerf

javascript/ql/lib/semmle/javascript/security/dataflow/IndirectCommandInjectionCustomizations.qll

@@ -49,6 +49,38 @@ module IndirectCommandInjection {
     override string describe() { result = "environment variable" }
   }
 
+  /** Gets a data flow node referring to `process.env`. */
+  private DataFlow::SourceNode envObject(DataFlow::TypeTracker t) {
+    t.start() and
+    result = NodeJSLib::process().getAPropertyRead("env")
+    or
+    exists(DataFlow::TypeTracker t2 | result = envObject(t2).track(t2, t))
+  }
+
+  /** Gets a data flow node referring to `process.env`. */
+  DataFlow::SourceNode envObject() { result = envObject(DataFlow::TypeTracker::end()) }
+
+  /**
+   * Gets the name of an environment variable that is assumed to be safe.
+   */
+  private string getASafeEnvironmentVariable() {
+    result =
+      [
+        "GITHUB_ACTION", "GITHUB_ACTION_PATH", "GITHUB_ACTION_REPOSITORY", "GITHUB_ACTIONS",
+        "GITHUB_ACTOR", "GITHUB_API_URL", "GITHUB_BASE_REF", "GITHUB_ENV", "GITHUB_EVENT_NAME",
+        "GITHUB_EVENT_PATH", "GITHUB_GRAPHQL_URL", "GITHUB_JOB", "GITHUB_PATH", "GITHUB_REF",
+        "GITHUB_REPOSITORY", "GITHUB_REPOSITORY_OWNER", "GITHUB_RUN_ID", "GITHUB_RUN_NUMBER",
+        "GITHUB_SERVER_URL", "GITHUB_SHA", "GITHUB_WORKFLOW", "GITHUB_WORKSPACE"
+      ]
+  }
+
+  /** Sanitizer that blocks flow through safe environment variables. */
+  private class SafeEnvVariableSanitizer extends Sanitizer {
+    SafeEnvVariableSanitizer() {
+      this = envObject().getAPropertyRead(getASafeEnvironmentVariable())
+    }
+  }
+
   /**
    * An object containing parsed command-line arguments, considered as a flow source for command injection.
    */

javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/IndirectCommandInjection.expected

@@ -1,4 +1,14 @@
 nodes
+| actions.js:3:6:3:16 | process.env |
+| actions.js:3:6:3:16 | process.env |
+| actions.js:3:6:3:29 | process ... _DATA'] |
+| actions.js:3:6:3:29 | process ... _DATA'] |
+| actions.js:6:15:6:15 | e |
+| actions.js:7:10:7:10 | e |
+| actions.js:7:10:7:23 | e['TEST_DATA'] |
+| actions.js:7:10:7:23 | e['TEST_DATA'] |
+| actions.js:11:6:11:16 | process.env |
+| actions.js:11:6:11:16 | process.env |
 | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
 | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
 | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
@@ -212,6 +222,15 @@ nodes
 | command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType |
 | command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType |
 edges
+| actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] |
+| actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] |
+| actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] |
+| actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] |
+| actions.js:6:15:6:15 | e | actions.js:7:10:7:10 | e |
+| actions.js:7:10:7:10 | e | actions.js:7:10:7:23 | e['TEST_DATA'] |
+| actions.js:7:10:7:10 | e | actions.js:7:10:7:23 | e['TEST_DATA'] |
+| actions.js:11:6:11:16 | process.env | actions.js:6:15:6:15 | e |
+| actions.js:11:6:11:16 | process.env | actions.js:6:15:6:15 | e |
 | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv |
 | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:22:8:36 | process.argv[2] |
 | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:22:8:36 | process.argv[2] |
@@ -400,6 +419,8 @@ edges
 | command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType | command-line-parameter-command-injection.js:146:10:146:38 | "cmd.sh ... zzaType |
 | command-line-parameter-command-injection.js:146:22:146:38 | program.pizzaType | command-line-parameter-command-injection.js:146:10:146:38 | "cmd.sh ... zzaType |
 #select
+| actions.js:3:6:3:29 | process ... _DATA'] | actions.js:3:6:3:16 | process.env | actions.js:3:6:3:29 | process ... _DATA'] | This command depends on an unsanitized $@. | actions.js:3:6:3:16 | process.env | environment variable |
+| actions.js:7:10:7:23 | e['TEST_DATA'] | actions.js:11:6:11:16 | process.env | actions.js:7:10:7:23 | e['TEST_DATA'] | This command depends on an unsanitized $@. | actions.js:11:6:11:16 | process.env | environment variable |
 | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:4:10:4:21 | process.argv | command-line argument |
 | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line-parameter-command-injection.js:8:10:8:36 | "cmd.sh ... argv[2] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:8:22:8:33 | process.argv | command-line argument |
 | command-line-parameter-command-injection.js:11:14:11:20 | args[0] | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line-parameter-command-injection.js:11:14:11:20 | args[0] | This command depends on an unsanitized $@. | command-line-parameter-command-injection.js:10:13:10:24 | process.argv | command-line argument |

javascript/ql/test/query-tests/Security/CWE-078/IndirectCommandInjection/actions.js

@@ -0,0 +1,11 @@
+import { exec } from "@actions/exec";
+
+exec(process.env['TEST_DATA']); // NOT OK
+exec(process.env['GITHUB_ACTION']); // OK
+
+function test(e) {
+    exec(e['TEST_DATA']); // NOT OK
+    exec(e['GITHUB_ACTION']); // OK
+}
+
+test(process.env);