Fix FP in envvar injection

JarLob · JarLob · commit 3d2053fab148 · 2026-07-27T13:51:31.000+03:00
diff --git a/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll b/actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll
@@ -151,7 +151,7 @@ Event getRelevantNonArtifactEventInPrivilegedContext(DataFlow::Node sink) {
 private module EnvVarInjectionConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
     source instanceof RemoteFlowSource and
-    not source.(RemoteFlowSource).getSourceType() = ["branch", "username"]
+    not source.(RemoteFlowSource).getSourceType() = ["branch", "label", "username"]
   }
 
   predicate isSink(DataFlow::Node sink) { sink instanceof EnvVarInjectionSink }
diff --git a/actions/ql/src/Security/CWE-077/EnvVarInjectionCritical.ql b/actions/ql/src/Security/CWE-077/EnvVarInjectionCritical.ql
@@ -22,6 +22,7 @@ import codeql.actions.security.ControlChecks
 from EnvVarInjectionFlow::PathNode source, EnvVarInjectionFlow::PathNode sink, Event event
 where
   EnvVarInjectionFlow::flowPath(source, sink) and
+  source.getNode().(RemoteFlowSource).getEventName() = event.getName() and
   // exclude paths to file read sinks from non-artifact sources
   (
     // source is text
diff --git a/actions/ql/src/change-notes/2026-07-18-envvar-injection-precision.md b/actions/ql/src/change-notes/2026-07-18-envvar-injection-precision.md
@@ -0,0 +1,4 @@
+---
+category: minorAnalysis
+---
+* The `actions/envvar-injection/critical` query now requires the untrusted source and privileged context to originate from the same trigger event. The environment variable injection queries also no longer treat pull request head labels as injection-capable because they cannot contain newlines.
diff --git a/actions/ql/test/query-tests/Security/CWE-077/.github/workflows/test18.yml b/actions/ql/test/query-tests/Security/CWE-077/.github/workflows/test18.yml
@@ -8,6 +8,8 @@ on:
 
 jobs:
   tests:
+    permissions:
+      contents: write
     runs-on: ubuntu-latest
     steps:
       - name: Checkout Repository
@@ -30,3 +32,7 @@ jobs:
           # Delete non-alphanumeric characters and limit to 75 chars which is the branch title limit in GitHub
           SAFE_PULL_REQUEST_TITLE=$(echo "${GITHUB_EVENT_PULL_REQUEST_TITLE}" | tr -cd '[:alnum:]_ -' | cut -c1-75)
           echo "SAFE_PULL_REQUEST_TITLE=$SAFE_PULL_REQUEST_TITLE" >> $GITHUB_ENV
+      - name: Keep source and privilege events correlated
+        env:
+          BODY: ${{ github.event.pull_request.body }}
+        run: echo "BODY=$BODY" >> $GITHUB_ENV
diff --git a/actions/ql/test/query-tests/Security/CWE-077/.github/workflows/test4.yml b/actions/ql/test/query-tests/Security/CWE-077/.github/workflows/test4.yml
@@ -66,6 +66,10 @@ jobs:
           ${TITLE}
           EOL
           echo REPO_NAME=$(cat issue.txt | sed 's/\r/\n/g' | grep -ioE '\s*[a-z0-9_-]+/[a-z0-9_-]+\s*$' | tr -d ' ') >> $GITHUB_ENV
+      - env:
+          LABEL: ${{ github.event.pull_request.head.label }}
+        run: |
+          echo "PR_LABEL=$LABEL" >> $GITHUB_ENV
 
 
 
diff --git a/actions/ql/test/query-tests/Security/CWE-077/EnvVarInjectionCritical.expected b/actions/ql/test/query-tests/Security/CWE-077/EnvVarInjectionCritical.expected
@@ -29,6 +29,7 @@ edges
 | .github/workflows/test12.yml:55:9:61:6 | Uses Step | .github/workflows/test12.yml:63:14:68:29 | {\n  echo 'PRERELEASE_REPORT<<EOF'\n  cat prerelease-report.md\n  echo EOF\n} >> "$GITHUB_ENV"\n | provenance | Config |
 | .github/workflows/test16.yml:10:9:15:6 | Uses Step | .github/workflows/test16.yml:15:14:17:63 | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENV\n | provenance | Config |
 | .github/workflows/test16.yml:10:9:15:6 | Uses Step | .github/workflows/test16.yml:18:14:20:77 | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt \| tr ',' '\\n')" >> $GITHUB_ENV\n | provenance | Config |
+| .github/workflows/test18.yml:37:18:37:54 | github.event.pull_request.body | .github/workflows/test18.yml:38:14:38:45 | echo "BODY=$BODY" >> $GITHUB_ENV | provenance | Config |
 nodes
 | .github/workflows/artifactpoisoning51.yml:13:9:15:6 | Run Step | semmle.label | Run Step |
 | .github/workflows/artifactpoisoning51.yml:19:14:20:57 | echo "pr_number=$(cat foo/bar)" >> $GITHUB_ENV\n | semmle.label | echo "pr_number=$(cat foo/bar)" >> $GITHUB_ENV\n |
@@ -91,6 +92,8 @@ nodes
 | .github/workflows/test16.yml:10:9:15:6 | Uses Step | semmle.label | Uses Step |
 | .github/workflows/test16.yml:15:14:17:63 | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENV\n | semmle.label | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENV\n |
 | .github/workflows/test16.yml:18:14:20:77 | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt \| tr ',' '\\n')" >> $GITHUB_ENV\n | semmle.label | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt \| tr ',' '\\n')" >> $GITHUB_ENV\n |
+| .github/workflows/test18.yml:37:18:37:54 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
+| .github/workflows/test18.yml:38:14:38:45 | echo "BODY=$BODY" >> $GITHUB_ENV | semmle.label | echo "BODY=$BODY" >> $GITHUB_ENV |
 subpaths
 #select
 | .github/workflows/artifactpoisoning51.yml:19:14:20:57 | echo "pr_number=$(cat foo/bar)" >> $GITHUB_ENV\n | .github/workflows/artifactpoisoning51.yml:13:9:15:6 | Run Step | .github/workflows/artifactpoisoning51.yml:19:14:20:57 | echo "pr_number=$(cat foo/bar)" >> $GITHUB_ENV\n | Potential environment variable injection in $@, which may be controlled by an external user ($@). | .github/workflows/artifactpoisoning51.yml:19:14:20:57 | echo "pr_number=$(cat foo/bar)" >> $GITHUB_ENV\n | echo "pr_number=$(cat foo/bar)" >> $GITHUB_ENV\n | .github/workflows/artifactpoisoning51.yml:4:3:4:14 | workflow_run | workflow_run |
diff --git a/actions/ql/test/query-tests/Security/CWE-077/EnvVarInjectionMedium.expected b/actions/ql/test/query-tests/Security/CWE-077/EnvVarInjectionMedium.expected
@@ -29,6 +29,7 @@ edges
 | .github/workflows/test12.yml:55:9:61:6 | Uses Step | .github/workflows/test12.yml:63:14:68:29 | {\n  echo 'PRERELEASE_REPORT<<EOF'\n  cat prerelease-report.md\n  echo EOF\n} >> "$GITHUB_ENV"\n | provenance | Config |
 | .github/workflows/test16.yml:10:9:15:6 | Uses Step | .github/workflows/test16.yml:15:14:17:63 | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENV\n | provenance | Config |
 | .github/workflows/test16.yml:10:9:15:6 | Uses Step | .github/workflows/test16.yml:18:14:20:77 | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt \| tr ',' '\\n')" >> $GITHUB_ENV\n | provenance | Config |
+| .github/workflows/test18.yml:37:18:37:54 | github.event.pull_request.body | .github/workflows/test18.yml:38:14:38:45 | echo "BODY=$BODY" >> $GITHUB_ENV | provenance | Config |
 nodes
 | .github/workflows/artifactpoisoning51.yml:13:9:15:6 | Run Step | semmle.label | Run Step |
 | .github/workflows/artifactpoisoning51.yml:19:14:20:57 | echo "pr_number=$(cat foo/bar)" >> $GITHUB_ENV\n | semmle.label | echo "pr_number=$(cat foo/bar)" >> $GITHUB_ENV\n |
@@ -91,5 +92,7 @@ nodes
 | .github/workflows/test16.yml:10:9:15:6 | Uses Step | semmle.label | Uses Step |
 | .github/workflows/test16.yml:15:14:17:63 | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENV\n | semmle.label | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt)" >> $GITHUB_ENV\n |
 | .github/workflows/test16.yml:18:14:20:77 | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt \| tr ',' '\\n')" >> $GITHUB_ENV\n | semmle.label | # VULNERABLE\necho "PR_NUMBER=$(cat pr_number.txt \| tr ',' '\\n')" >> $GITHUB_ENV\n |
+| .github/workflows/test18.yml:37:18:37:54 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
+| .github/workflows/test18.yml:38:14:38:45 | echo "BODY=$BODY" >> $GITHUB_ENV | semmle.label | echo "BODY=$BODY" >> $GITHUB_ENV |
 subpaths
 #select

Original file line number	Diff line number	Diff line change
`@@ -151,7 +151,7 @@ Event getRelevantNonArtifactEventInPrivilegedContext(DataFlow::Node sink) {`
`151`	`151`	`private module EnvVarInjectionConfig implements DataFlow::ConfigSig {`
`152`	`152`	`predicate isSource(DataFlow::Node source) {`
`153`	`153`	`source instanceof RemoteFlowSource and`
`154`		`- not source.(RemoteFlowSource).getSourceType() = ["branch", "username"]`
	`154`	`+ not source.(RemoteFlowSource).getSourceType() = ["branch", "label", "username"]`
`155`	`155`	`}`
`156`	`156`
`157`	`157`	`predicate isSink(DataFlow::Node sink) { sink instanceof EnvVarInjectionSink }`
Original file line number	Diff line number	Diff line change
`@@ -22,6 +22,7 @@ import codeql.actions.security.ControlChecks`
`22`	`22`	`from EnvVarInjectionFlow::PathNode source, EnvVarInjectionFlow::PathNode sink, Event event`
`23`	`23`	`where`
`24`	`24`	`EnvVarInjectionFlow::flowPath(source, sink) and`
	`25`	`+ source.getNode().(RemoteFlowSource).getEventName() = event.getName() and`
`25`	`26`	`// exclude paths to file read sinks from non-artifact sources`
`26`	`27`	`(`
`27`	`28`	`// source is text`
-Original file line number
+Diff line change
@@ @@ -0,0 +1,4 @@ @@
 +---
 +category: minorAnalysis
 +---
 +* The `actions/envvar-injection/critical` query now requires the untrusted source and privileged context to originate from the same trigger event. The environment variable injection queries also no longer treat pull request head labels as injection-capable because they cannot contain newlines.