Add simple taint tracking for env variables

javascript/ql/lib/semmle/javascript/Actions.qll

@@ -28,6 +28,9 @@ module Actions {
     /** Gets the `jobs` mapping from job IDs to job definitions in this workflow. */
     YamlMapping getJobs() { result = this.lookup("jobs") }
 
+    /** Gets the 'global' `env` mapping in this workflow. */
+    YamlMapping getEnv() { result = this.lookup("env") }
+
     /** Gets the name of the workflow. */
     string getName() { result = this.lookup("name").(YamlString).getValue() }
 
@@ -54,6 +57,54 @@ module Actions {
     Workflow getWorkflow() { result = workflow }
   }
 
+  /** An environment variable in 'env:' */
+  abstract class Env extends YamlNode, YamlString {
+    /** Gets the name of this environment variable. */
+    abstract string getName();
+  }
+
+  /** Workflow level 'global' environment variable. */
+  class GlobalEnv extends Env {
+    string envName;
+    Workflow workflow;
+
+    GlobalEnv() { this = workflow.getEnv().lookup(envName) }
+
+    /** Gets the workflow this field belongs to. */
+    Workflow getWorkflow() { result = workflow }
+
+    /** Gets the name of this environment variable. */
+    override string getName() { result = envName }
+  }
+
+  /** Job level environment variable. */
+  class JobEnv extends Env {
+    string envName;
+    Job job;
+
+    JobEnv() { this = job.getEnv().lookup(envName) }
+
+    /** Gets the job this field belongs to. */
+    Job getJob() { result = job }
+
+    /** Gets the name of this environment variable. */
+    override string getName() { result = envName }
+  }
+
+  /** Step level environment variable. */
+  class StepEnv extends Env {
+    string envName;
+    Step step;
+
+    StepEnv() { this = step.getEnv().lookup(envName) }
+
+    /** Gets the step this field belongs to. */
+    Step getStep() { result = step }
+
+    /** Gets the name of this environment variable. */
+    override string getName() { result = envName }
+  }
+
   /**
    * An Actions job within a workflow.
    * See https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions#jobs.
@@ -88,6 +139,9 @@ module Actions {
     /** Gets the sequence of `steps` within this job. */
     YamlSequence getSteps() { result = this.lookup("steps") }
 
+    /** Gets the `env` mapping in this job. */
+    YamlMapping getEnv() { result = this.lookup("env") }
+
     /** Gets the workflow this job belongs to. */
     Workflow getWorkflow() { result = workflow }
 
@@ -149,6 +203,9 @@ module Actions {
     /** Gets the value of the `if` field in this step, if any. */
     StepIf getIf() { result.getStep() = this }
 
+    /** Gets the value of the `env` field in this step, if any. */
+    YamlMapping getEnv() { result = this.lookup("env") }
+
     /** Gets the ID of this step, if any. */
     string getId() { result = this.lookup("id").(YamlString).getValue() }
   }
@@ -259,6 +316,10 @@ module Actions {
           .regexpCapture("\\$\\{\\{\\s*([A-Za-z0-9_\\[\\]\\*\\((\\)\\.\\-]+)\\s*\\}\\}", 1)
   }
 
+  /** Extracts the 'name' part from env.name */
+  bindingset[name]
+  string getEnvName(string name) { result = name.regexpCapture("env\\.([A-Za-z0-9_]+)", 1) }
+
   /**
    * A `script:` field within an Actions `with:` specific to `actions/github-script` action.
    *

javascript/ql/src/Security/CWE-094/ExpressionInjection.qhelp

@@ -8,10 +8,11 @@
 			code injection in contexts like <i>run:</i> or <i>script:</i>.
 		</p>
         <p>
-            Code injection in GitHub Actions may allow an attacker to 
-            exfiltrate the temporary GitHub repository authorization token. 
+            Code injection in GitHub Actions may allow an attacker to
+            exfiltrate any secrets used in the workflow and
+            the temporary GitHub repository authorization token.
             The token might have write access to the repository, allowing an attacker
-             to use the token to make changes to the repository.
+            to use the token to make changes to the repository.
         </p>
 	</overview>
 
@@ -19,7 +20,8 @@
 		<p>
 			The best practice to avoid code injection vulnerabilities
 			in GitHub workflows is to set the untrusted input value of the expression
-			to an intermediate environment variable.
+			to an intermediate environment variable and then use the environment variable
+			using the native syntax of the shell/script interpreter (i.e. <b>NOT</b> the ${{ env.VAR }}).
 		</p>
         <p>
             It is also recommended to limit the permissions of any tokens used 

javascript/ql/src/Security/CWE-094/ExpressionInjection.ql

@@ -103,22 +103,38 @@ private predicate isExternalUserControlledWorkflowRun(string context) {
   )
 }
 
-from YamlNode node, string context, Actions::On on
+from YamlNode node, string injection, string context, Actions::On on
 where
   (
     exists(Actions::Run run |
       node = run and
-      Actions::getASimpleReferenceExpression(run) = context and
-      run.getStep().getJob().getWorkflow().getOn() = on
+      Actions::getASimpleReferenceExpression(run) = injection and
+      run.getStep().getJob().getWorkflow().getOn() = on and
+      (
+        injection = context
+        or
+        exists(Actions::Env env |
+          Actions::getEnvName(injection) = env.getName() and
+          Actions::getASimpleReferenceExpression(env) = context
+        )
+      )
     )
     or
     exists(Actions::Script script, Actions::Step step, Actions::Uses uses |
       node = script and
+      script.getWith().getStep().getJob().getWorkflow().getOn() = on and
       script.getWith().getStep() = step and
       uses.getStep() = step and
       uses.getGitHubRepository() = "actions/github-script" and
-      Actions::getASimpleReferenceExpression(script) = context and
-      script.getWith().getStep().getJob().getWorkflow().getOn() = on
+      Actions::getASimpleReferenceExpression(script) = injection and
+      (
+        injection = context
+        or
+        exists(Actions::Env env |
+          Actions::getEnvName(injection) = env.getName() and
+          Actions::getASimpleReferenceExpression(env) = context
+        )
+      )
     )
   ) and
   (
@@ -153,5 +169,5 @@ where
     isExternalUserControlledWorkflowRun(context)
   )
 select node,
-  "Potential injection from the " + context +
+  "Potential injection from the " + injection +
     " context, which may be controlled by an external user."

javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/issues.yml

@@ -1,8 +1,20 @@
 on: issues
 
+env:
+  global_env: ${{ github.event.issue.title }}
+  test: test
+
 jobs:
   echo-chamber:
+    env:
+      job_env: ${{ github.event.issue.title }}
     runs-on: ubuntu-latest
     steps:
     - run: echo '${{ github.event.issue.title }}'
     - run: echo '${{ github.event.issue.body }}'
+    - run: echo '${{ env.global_env }}'
+    - run: echo '${{ env.test }}'
+    - run: echo '${{ env.job_env }}'
+    - run: echo '${{ env.step_env }}'
+      env:
+        step_env: ${{ github.event.issue.title }}

javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.expected

@@ -15,8 +15,11 @@
 | .github/workflows/gollum.yml:8:12:8:53 | echo '$ ... tle }}' | Potential injection from the github.event.pages[11].title context, which may be controlled by an external user. |
 | .github/workflows/gollum.yml:9:12:9:56 | echo '$ ... ame }}' | Potential injection from the github.event.pages[0].page_name context, which may be controlled by an external user. |
 | .github/workflows/gollum.yml:10:12:10:59 | echo '$ ... ame }}' | Potential injection from the github.event.pages[2222].page_name context, which may be controlled by an external user. |
-| .github/workflows/issues.yml:7:12:7:49 | echo '$ ... tle }}' | Potential injection from the github.event.issue.title context, which may be controlled by an external user. |
-| .github/workflows/issues.yml:8:12:8:48 | echo '$ ... ody }}' | Potential injection from the github.event.issue.body context, which may be controlled by an external user. |
+| .github/workflows/issues.yml:13:12:13:49 | echo '$ ... tle }}' | Potential injection from the github.event.issue.title context, which may be controlled by an external user. |
+| .github/workflows/issues.yml:14:12:14:48 | echo '$ ... ody }}' | Potential injection from the github.event.issue.body context, which may be controlled by an external user. |
+| .github/workflows/issues.yml:15:12:15:39 | echo '$ ... env }}' | Potential injection from the env.global_env context, which may be controlled by an external user. |
+| .github/workflows/issues.yml:17:12:17:36 | echo '$ ... env }}' | Potential injection from the env.job_env context, which may be controlled by an external user. |
+| .github/workflows/issues.yml:18:12:18:37 | echo '$ ... env }}' | Potential injection from the env.step_env context, which may be controlled by an external user. |
 | .github/workflows/pull_request_review.yml:7:12:7:56 | echo '$ ... tle }}' | Potential injection from the github.event.pull_request.title context, which may be controlled by an external user. |
 | .github/workflows/pull_request_review.yml:8:12:8:55 | echo '$ ... ody }}' | Potential injection from the github.event.pull_request.body context, which may be controlled by an external user. |
 | .github/workflows/pull_request_review.yml:9:12:9:61 | echo '$ ... bel }}' | Potential injection from the github.event.pull_request.head.label context, which may be controlled by an external user. |