Skip to content

Commit 0fb9774

Browse files
authored
Merge pull request #22235 from JarLob/fix/actions-envvar-injection-precision
Actions: Fix FP in envvar injection
2 parents b078c16 + de68881 commit 0fb9774

7 files changed

Lines changed: 28 additions & 1 deletion

File tree

actions/ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ Event getRelevantNonArtifactEventInPrivilegedContext(DataFlow::Node sink) {
151151
private module EnvVarInjectionConfig implements DataFlow::ConfigSig {
152152
predicate isSource(DataFlow::Node source) {
153153
source instanceof RemoteFlowSource and
154-
not source.(RemoteFlowSource).getSourceType() = ["branch", "username"]
154+
not source.(RemoteFlowSource).getSourceType() = ["branch", "label", "username"]
155155
}
156156

157157
predicate isSink(DataFlow::Node sink) { sink instanceof EnvVarInjectionSink }

actions/ql/src/Security/CWE-077/EnvVarInjectionCritical.ql

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -19,9 +19,16 @@ import codeql.actions.dataflow.FlowSources
1919
import EnvVarInjectionFlow::PathGraph
2020
import codeql.actions.security.ControlChecks
2121

22+
bindingset[source, event]
23+
pragma[inline_late]
24+
private predicate hasSameEventName(RemoteFlowSource source, Event event) {
25+
source.getEventName() = event.getName()
26+
}
27+
2228
from EnvVarInjectionFlow::PathNode source, EnvVarInjectionFlow::PathNode sink, Event event
2329
where
2430
EnvVarInjectionFlow::flowPath(source, sink) and
31+
hasSameEventName(source.getNode(), event) and
2532
// exclude paths to file read sinks from non-artifact sources
2633
(
2734
// source is text
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
4+
* 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.

actions/ql/test/query-tests/Security/CWE-077/.github/workflows/test18.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,8 @@ on:
88

99
jobs:
1010
tests:
11+
permissions:
12+
contents: write
1113
runs-on: ubuntu-latest
1214
steps:
1315
- name: Checkout Repository
@@ -30,3 +32,7 @@ jobs:
3032
# Delete non-alphanumeric characters and limit to 75 chars which is the branch title limit in GitHub
3133
SAFE_PULL_REQUEST_TITLE=$(echo "${GITHUB_EVENT_PULL_REQUEST_TITLE}" | tr -cd '[:alnum:]_ -' | cut -c1-75)
3234
echo "SAFE_PULL_REQUEST_TITLE=$SAFE_PULL_REQUEST_TITLE" >> $GITHUB_ENV
35+
- name: Keep source and privilege events correlated
36+
env:
37+
BODY: ${{ github.event.pull_request.body }}
38+
run: echo "BODY=$BODY" >> $GITHUB_ENV

actions/ql/test/query-tests/Security/CWE-077/.github/workflows/test4.yml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,10 @@ jobs:
6666
${TITLE}
6767
EOL
6868
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
69+
- env:
70+
LABEL: ${{ github.event.pull_request.head.label }}
71+
run: |
72+
echo "PR_LABEL=$LABEL" >> $GITHUB_ENV
6973
7074
7175

actions/ql/test/query-tests/Security/CWE-077/EnvVarInjectionCritical.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ edges
2929
| .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 |
3030
| .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 |
3131
| .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 |
32+
| .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 |
3233
nodes
3334
| .github/workflows/artifactpoisoning51.yml:13:9:15:6 | Run Step | semmle.label | Run Step |
3435
| .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
9192
| .github/workflows/test16.yml:10:9:15:6 | Uses Step | semmle.label | Uses Step |
9293
| .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 |
9394
| .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 |
95+
| .github/workflows/test18.yml:37:18:37:54 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
96+
| .github/workflows/test18.yml:38:14:38:45 | echo "BODY=$BODY" >> $GITHUB_ENV | semmle.label | echo "BODY=$BODY" >> $GITHUB_ENV |
9497
subpaths
9598
#select
9699
| .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 |

actions/ql/test/query-tests/Security/CWE-077/EnvVarInjectionMedium.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ edges
2929
| .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 |
3030
| .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 |
3131
| .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 |
32+
| .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 |
3233
nodes
3334
| .github/workflows/artifactpoisoning51.yml:13:9:15:6 | Run Step | semmle.label | Run Step |
3435
| .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
9192
| .github/workflows/test16.yml:10:9:15:6 | Uses Step | semmle.label | Uses Step |
9293
| .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 |
9394
| .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 |
95+
| .github/workflows/test18.yml:37:18:37:54 | github.event.pull_request.body | semmle.label | github.event.pull_request.body |
96+
| .github/workflows/test18.yml:38:14:38:45 | echo "BODY=$BODY" >> $GITHUB_ENV | semmle.label | echo "BODY=$BODY" >> $GITHUB_ENV |
9497
subpaths
9598
#select

0 commit comments

Comments
 (0)