Skip to content

Commit cb69780

Browse files
authored
Merge pull request #19388 from AdnaneKhan/patch-1
Actions: Fix Critical Artifact poisoning False Positive
2 parents d82d5c2 + c95b5ce commit cb69780

File tree

11 files changed

+119
-2
lines changed

11 files changed

+119
-2
lines changed
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: fix
3+
---
4+
* The `actions/artifact-poisoning/critical` and `actions/artifact-poisoning/medium` queries now exclude artifacts downloaded to `$[{ runner.temp }}` in addition to `/tmp`.

actions/ql/lib/codeql/actions/Helper.qll

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ string normalizePath(string path) {
7272
then result = path
7373
else
7474
// foo -> GITHUB_WORKSPACE/foo
75-
if path.regexpMatch("^[^/~].*")
75+
if path.regexpMatch("^[^$/~].*")
7676
then result = "GITHUB_WORKSPACE/" + path.regexpReplaceAll("/$", "")
7777
else
7878
// ~/foo -> ~/foo

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -262,8 +262,10 @@ class ArtifactPoisoningSink extends DataFlow::Node {
262262

263263
ArtifactPoisoningSink() {
264264
download.getAFollowingStep() = poisonable and
265-
// excluding artifacts downloaded to /tmp
265+
// excluding artifacts downloaded to the temporary directory
266266
not download.getPath().regexpMatch("^/tmp.*") and
267+
not download.getPath().regexpMatch("^\\$\\{\\{\\s*runner\\.temp\\s*}}.*") and
268+
not download.getPath().regexpMatch("^\\$RUNNER_TEMP.*") and
267269
(
268270
poisonable.(Run).getScript() = this.asExpr() and
269271
(
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
on:
2+
workflow_run:
3+
workflows:
4+
- Benchmark
5+
types:
6+
- completed
7+
8+
jobs:
9+
benchmark:
10+
runs-on: ubuntu-latest
11+
steps:
12+
- uses: actions/checkout@v4
13+
- name: Download From PR
14+
uses: actions/download-artifact@v4
15+
with:
16+
github-token: ${{ secrets.GITHUB_TOKEN }}
17+
run-id: ${{ github.event.workflow_run.id }}
18+
path: ${{ runner.temp }}/artifacts/
19+
- run: npm install
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
on:
2+
workflow_run:
3+
workflows:
4+
- Benchmark
5+
types:
6+
- completed
7+
8+
jobs:
9+
benchmark:
10+
runs-on: ubuntu-latest
11+
steps:
12+
- uses: actions/checkout@v4
13+
- name: Download From PR
14+
uses: actions/download-artifact@v4
15+
with:
16+
github-token: ${{ secrets.GITHUB_TOKEN }}
17+
run-id: ${{ github.event.workflow_run.id }}
18+
path: /tmp/artifacts/
19+
- run: npm install
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
on:
2+
workflow_run:
3+
workflows:
4+
- Benchmark
5+
types:
6+
- completed
7+
8+
jobs:
9+
benchmark:
10+
runs-on: ubuntu-latest
11+
steps:
12+
- uses: actions/checkout@v4
13+
- name: Download From PR
14+
uses: actions/download-artifact@v4
15+
with:
16+
github-token: ${{ secrets.GITHUB_TOKEN }}
17+
run-id: ${{ github.event.workflow_run.id }}
18+
path: $RUNNER_TEMP/artifacts/
19+
- run: npm install
Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
on:
2+
workflow_run:
3+
workflows:
4+
- Benchmark
5+
types:
6+
- completed
7+
8+
jobs:
9+
benchmark:
10+
runs-on: ubuntu-latest
11+
steps:
12+
- uses: actions/checkout@v4
13+
- name: Download From PR
14+
uses: actions/download-artifact@v4
15+
with:
16+
github-token: ${{ secrets.GITHUB_TOKEN }}
17+
run-id: ${{ github.event.workflow_run.id }}
18+
- run: npm install
Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
on:
2+
workflow_run:
3+
workflows:
4+
- Benchmark
5+
types:
6+
- completed
7+
8+
jobs:
9+
benchmark:
10+
runs-on: ubuntu-latest
11+
steps:
12+
- uses: actions/checkout@v4
13+
- name: Download From PR
14+
uses: actions/download-artifact@v4
15+
with:
16+
github-token: ${{ secrets.GITHUB_TOKEN }}
17+
run-id: ${{ github.event.workflow_run.id }}
18+
path: ${{ runner.temp }}/artifacts/
19+
- run: npm install

actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningCritical.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ edges
1313
| .github/workflows/artifactpoisoning42.yml:13:9:21:6 | Run Step | .github/workflows/artifactpoisoning42.yml:22:14:22:18 | ./cmd | provenance | Config |
1414
| .github/workflows/artifactpoisoning71.yml:9:9:16:6 | Uses Step | .github/workflows/artifactpoisoning71.yml:17:14:18:40 | sed -f config foo.md > bar.md\n | provenance | Config |
1515
| .github/workflows/artifactpoisoning81.yml:28:9:31:6 | Uses Step | .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | provenance | Config |
16+
| .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | provenance | Config |
1617
| .github/workflows/artifactpoisoning101.yml:10:9:16:6 | Uses Step | .github/workflows/artifactpoisoning101.yml:17:14:19:59 | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | provenance | Config |
1718
| .github/workflows/test18.yml:12:15:33:12 | Uses Step | .github/workflows/test18.yml:36:15:40:58 | Uses Step | provenance | Config |
1819
| .github/workflows/test25.yml:22:9:32:6 | Uses Step: downloadBuildScan | .github/workflows/test25.yml:39:14:40:45 | ./gradlew buildScanPublishPrevious\n | provenance | Config |
@@ -44,6 +45,8 @@ nodes
4445
| .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | semmle.label | python test.py |
4546
| .github/workflows/artifactpoisoning92.yml:28:9:29:6 | Uses Step | semmle.label | Uses Step |
4647
| .github/workflows/artifactpoisoning92.yml:29:14:29:26 | make snapshot | semmle.label | make snapshot |
48+
| .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | semmle.label | Uses Step |
49+
| .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | semmle.label | npm install |
4750
| .github/workflows/artifactpoisoning101.yml:10:9:16:6 | Uses Step | semmle.label | Uses Step |
4851
| .github/workflows/artifactpoisoning101.yml:17:14:19:59 | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | semmle.label | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n |
4952
| .github/workflows/test18.yml:12:15:33:12 | Uses Step | semmle.label | Uses Step |
@@ -66,6 +69,7 @@ subpaths
6669
| .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | .github/workflows/artifactpoisoning81.yml:28:9:31:6 | Uses Step | .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | Potential artifact poisoning in $@, which may be controlled by an external user ($@). | .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | python test.py | .github/workflows/artifactpoisoning81.yml:3:5:3:23 | pull_request_target | pull_request_target |
6770
| .github/workflows/artifactpoisoning92.yml:28:9:29:6 | Uses Step | .github/actions/download-artifact-2/action.yaml:6:7:25:4 | Uses Step | .github/workflows/artifactpoisoning92.yml:28:9:29:6 | Uses Step | Potential artifact poisoning in $@, which may be controlled by an external user ($@). | .github/workflows/artifactpoisoning92.yml:28:9:29:6 | Uses Step | Uses Step | .github/workflows/artifactpoisoning92.yml:3:3:3:14 | workflow_run | workflow_run |
6871
| .github/workflows/artifactpoisoning92.yml:29:14:29:26 | make snapshot | .github/actions/download-artifact-2/action.yaml:6:7:25:4 | Uses Step | .github/workflows/artifactpoisoning92.yml:29:14:29:26 | make snapshot | Potential artifact poisoning in $@, which may be controlled by an external user ($@). | .github/workflows/artifactpoisoning92.yml:29:14:29:26 | make snapshot | make snapshot | .github/workflows/artifactpoisoning92.yml:3:3:3:14 | workflow_run | workflow_run |
72+
| .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | Potential artifact poisoning in $@, which may be controlled by an external user ($@). | .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | npm install | .github/workflows/artifactpoisoning96.yml:2:3:2:14 | workflow_run | workflow_run |
6973
| .github/workflows/artifactpoisoning101.yml:17:14:19:59 | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | .github/workflows/artifactpoisoning101.yml:10:9:16:6 | Uses Step | .github/workflows/artifactpoisoning101.yml:17:14:19:59 | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | Potential artifact poisoning in $@, which may be controlled by an external user ($@). | .github/workflows/artifactpoisoning101.yml:17:14:19:59 | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | .github/workflows/artifactpoisoning101.yml:4:3:4:21 | pull_request_target | pull_request_target |
7074
| .github/workflows/test18.yml:36:15:40:58 | Uses Step | .github/workflows/test18.yml:12:15:33:12 | Uses Step | .github/workflows/test18.yml:36:15:40:58 | Uses Step | Potential artifact poisoning in $@, which may be controlled by an external user ($@). | .github/workflows/test18.yml:36:15:40:58 | Uses Step | Uses Step | .github/workflows/test18.yml:3:5:3:16 | workflow_run | workflow_run |
7175
| .github/workflows/test25.yml:39:14:40:45 | ./gradlew buildScanPublishPrevious\n | .github/workflows/test25.yml:22:9:32:6 | Uses Step: downloadBuildScan | .github/workflows/test25.yml:39:14:40:45 | ./gradlew buildScanPublishPrevious\n | Potential artifact poisoning in $@, which may be controlled by an external user ($@). | .github/workflows/test25.yml:39:14:40:45 | ./gradlew buildScanPublishPrevious\n | ./gradlew buildScanPublishPrevious\n | .github/workflows/test25.yml:2:3:2:14 | workflow_run | workflow_run |

actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningMedium.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ edges
1313
| .github/workflows/artifactpoisoning42.yml:13:9:21:6 | Run Step | .github/workflows/artifactpoisoning42.yml:22:14:22:18 | ./cmd | provenance | Config |
1414
| .github/workflows/artifactpoisoning71.yml:9:9:16:6 | Uses Step | .github/workflows/artifactpoisoning71.yml:17:14:18:40 | sed -f config foo.md > bar.md\n | provenance | Config |
1515
| .github/workflows/artifactpoisoning81.yml:28:9:31:6 | Uses Step | .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | provenance | Config |
16+
| .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | provenance | Config |
1617
| .github/workflows/artifactpoisoning101.yml:10:9:16:6 | Uses Step | .github/workflows/artifactpoisoning101.yml:17:14:19:59 | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | provenance | Config |
1718
| .github/workflows/test18.yml:12:15:33:12 | Uses Step | .github/workflows/test18.yml:36:15:40:58 | Uses Step | provenance | Config |
1819
| .github/workflows/test25.yml:22:9:32:6 | Uses Step: downloadBuildScan | .github/workflows/test25.yml:39:14:40:45 | ./gradlew buildScanPublishPrevious\n | provenance | Config |
@@ -44,6 +45,8 @@ nodes
4445
| .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | semmle.label | python test.py |
4546
| .github/workflows/artifactpoisoning92.yml:28:9:29:6 | Uses Step | semmle.label | Uses Step |
4647
| .github/workflows/artifactpoisoning92.yml:29:14:29:26 | make snapshot | semmle.label | make snapshot |
48+
| .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | semmle.label | Uses Step |
49+
| .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | semmle.label | npm install |
4750
| .github/workflows/artifactpoisoning101.yml:10:9:16:6 | Uses Step | semmle.label | Uses Step |
4851
| .github/workflows/artifactpoisoning101.yml:17:14:19:59 | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | semmle.label | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n |
4952
| .github/workflows/test18.yml:12:15:33:12 | Uses Step | semmle.label | Uses Step |

0 commit comments

Comments
 (0)