From 40a6f3bbee8348d2b2f0e66b510236bcd91b1ec7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= <pwntester@github.com>
Date: Thu, 27 Jun 2024 22:53:55 +0200
Subject: [PATCH] Make EnvVar and Path injection equivalent

---
 .../security/EnvPathInjectionQuery.qll        |  5 ++-
 .../actions/security/EnvVarInjectionQuery.qll | 39 ++++++++++++-------
 2 files changed, 29 insertions(+), 15 deletions(-)

diff --git a/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll b/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll
index cd049cccf4e..453966f0101 100644
--- a/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll
+++ b/ql/lib/codeql/actions/security/EnvPathInjectionQuery.qll
@@ -15,7 +15,8 @@ class EnvPathInjectionFromFileReadSink extends EnvPathInjectionSink {
       writeToGitHubPath(run, value) and
       // (eg: echo DATABASE_SHA=`yq '.creationMetadata.sha' codeql-database.yml` >> $GITHUB_ENV)
       value
-          .regexpMatch(["\\$\\(", "`"] + ["cat\\s+", "<", "jq\\s+", "yq\\s+"] + ".*" + ["`", "\\)"])
+          .regexpMatch(["\\$\\(", "`"] +
+              ["cat\\s+", "<", "jq\\s+", "yq\\s+", "tail\\s+", "head\\s+"] + ".*" + ["`", "\\)"])
     )
   }
 }
@@ -31,7 +32,7 @@ class EnvPathInjectionFromFileReadSink extends EnvPathInjectionSink {
 class EnvPathInjectionFromEnvVarSink extends EnvPathInjectionSink {
   EnvPathInjectionFromEnvVarSink() {
     exists(Run run, Expression expr, string var_name, string value |
-      this.asExpr().getInScopeEnvVarExpr(var_name) = expr and
+      run.getInScopeEnvVarExpr(var_name) = expr and
       run.getScriptScalar() = this.asExpr() and
       writeToGitHubPath(run, value) and
       (
diff --git a/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll b/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll
index a692c6e5874..a78963086e1 100644
--- a/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll
+++ b/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll
@@ -7,18 +7,6 @@ import codeql.actions.DataFlow
 
 abstract class EnvVarInjectionSink extends DataFlow::Node { }
 
-class EnvVarInjectionFromEnvVarSink extends EnvVarInjectionSink {
-  EnvVarInjectionFromEnvVarSink() {
-    exists(Run run, Expression expr, string var_name, string content, string value |
-      expr = run.getInScopeEnvVarExpr(var_name) and
-      writeToGitHubEnv(run, content) and
-      extractVariableAndValue(content, _, value) and
-      run.getScriptScalar() = this.asExpr() and
-      value.matches("%$" + ["", "{", "ENV{"] + var_name + "%")
-    )
-  }
-}
-
 class EnvVarInjectionFromFileReadSink extends EnvVarInjectionSink {
   EnvVarInjectionFromFileReadSink() {
     exists(Run run, UntrustedArtifactDownloadStep step, string content, string value |
@@ -28,7 +16,32 @@ class EnvVarInjectionFromFileReadSink extends EnvVarInjectionSink {
       extractVariableAndValue(content, _, value) and
       // (eg: echo DATABASE_SHA=`yq '.creationMetadata.sha' codeql-database.yml` >> $GITHUB_ENV)
       value
-          .regexpMatch(["\\$\\(", "`"] + ["cat\\s+", "<", "jq\\s+", "yq\\s+"] + ".*" + ["`", "\\)"])
+          .regexpMatch(["\\$\\(", "`"] +
+              ["cat\\s+", "<", "jq\\s+", "yq\\s+", "tail\\s+", "head\\s+"] + ".*" + ["`", "\\)"])
+    )
+  }
+}
+
+/**
+ * Holds if a Run step declares an environment variable, uses it to declare env var.
+ * e.g.
+ *    env:
+ *      BODY: ${{ github.event.comment.body }}
+ *    run: |
+ *      echo "FOO=$BODY" >> $GITHUB_ENV
+ */
+class EnvVarInjectionFromEnvVarSink extends EnvVarInjectionSink {
+  EnvVarInjectionFromEnvVarSink() {
+    exists(Run run, Expression expr, string var_name, string content, string value |
+      run.getInScopeEnvVarExpr(var_name) = expr and
+      run.getScriptScalar() = this.asExpr() and
+      writeToGitHubEnv(run, content) and
+      extractVariableAndValue(content, _, value) and
+      (
+        value.matches("%$" + ["", "{", "ENV{"] + var_name + "%")
+        or
+        value.matches("$(echo %") and value.indexOf(var_name) > 0
+      )
     )
   }
 }