From 38f00775bd87f6c1f27608dc23e072671c07cda5 Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Fri, 25 Apr 2025 14:49:01 -0400 Subject: [PATCH 1/9] Exclude artifacts downloaded to runner temp. --- .../ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll index d8d5f83c867d..24e0f400e920 100644 --- a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll +++ b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll @@ -262,8 +262,9 @@ class ArtifactPoisoningSink extends DataFlow::Node { ArtifactPoisoningSink() { download.getAFollowingStep() = poisonable and - // excluding artifacts downloaded to /tmp + // excluding artifacts downloaded to /tmp and runner.tmp not download.getPath().regexpMatch("^/tmp.*") and + not download.getPath().regexpMatch("^\${{\s?runner.temp\s?}}.*") and ( poisonable.(Run).getScript() = this.asExpr() and ( From a9c4d6f383c68df3491fb6537519139aacee7681 Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Fri, 25 Apr 2025 15:00:14 -0400 Subject: [PATCH 2/9] Fix escaping. --- .../ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll index 24e0f400e920..8c6471b3c580 100644 --- a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll +++ b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll @@ -264,7 +264,7 @@ class ArtifactPoisoningSink extends DataFlow::Node { download.getAFollowingStep() = poisonable and // excluding artifacts downloaded to /tmp and runner.tmp not download.getPath().regexpMatch("^/tmp.*") and - not download.getPath().regexpMatch("^\${{\s?runner.temp\s?}}.*") and + not download.getPath().regexpMatch("^\\${{\\s?runner.temp\\s?}}.*") and ( poisonable.(Run).getScript() = this.asExpr() and ( From f4f919635a7ee0eb9accf3c76138282cc758c23b Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Tue, 8 Jul 2025 10:17:29 -0400 Subject: [PATCH 3/9] Correctly specify regex. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jaroslav Lobačevski --- .../ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll index 8c6471b3c580..0db3b2f7f887 100644 --- a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll +++ b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll @@ -264,7 +264,7 @@ class ArtifactPoisoningSink extends DataFlow::Node { download.getAFollowingStep() = poisonable and // excluding artifacts downloaded to /tmp and runner.tmp not download.getPath().regexpMatch("^/tmp.*") and - not download.getPath().regexpMatch("^\\${{\\s?runner.temp\\s?}}.*") and + not download.getPath().regexpMatch("^\\$\\{\\{\\s?runner\\.temp\\s?}}.*") and ( poisonable.(Run).getScript() = this.asExpr() and ( From 5d6a5d5cbbe5f15852f4fa5778d7fb094edfdd7f Mon Sep 17 00:00:00 2001 From: AdnaneKhan Date: Tue, 8 Jul 2025 10:35:39 -0400 Subject: [PATCH 4/9] Add change notes and test workflow file. --- actions/ql/lib/change-notes/2025-07-08.md | 4 ++++ .../.github/workflows/artifactpoison93.yml | 22 +++++++++++++++++++ 2 files changed, 26 insertions(+) create mode 100644 actions/ql/lib/change-notes/2025-07-08.md create mode 100644 actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoison93.yml diff --git a/actions/ql/lib/change-notes/2025-07-08.md b/actions/ql/lib/change-notes/2025-07-08.md new file mode 100644 index 000000000000..893a695a22c6 --- /dev/null +++ b/actions/ql/lib/change-notes/2025-07-08.md @@ -0,0 +1,4 @@ +--- +category: fix +--- +* The `actions/artifact-poisoning/critical` and `actions/artifact-poisoning/medium` queries now exclude artifacts downloaded to `$[{ runner.temp }}` in addition to `/tmp`. diff --git a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoison93.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoison93.yml new file mode 100644 index 000000000000..d463527f292c --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoison93.yml @@ -0,0 +1,22 @@ +name: Secure Workflow + +on: + workflow_run: + workflows: ["Prev"] + types: + - completed + +jobs: + Download: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v2 + - run: mkdir -p ${{ runner.temp }}/artifacts/ + - uses: dawidd6/action-download-artifact@v2 + with: + name: pr_number + path: ${{ runner.temp }}/artifacts/ + + - name: Run command + run: | + sh cmd.sh From 9393181c4e714fa4d950dbd86c4540e99a49798d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jaroslav=20Loba=C4=8Devski?= Date: Tue, 8 Jul 2025 16:18:12 +0000 Subject: [PATCH 5/9] Add tests and path normalization fix to handle $ expansion --- actions/ql/lib/codeql/actions/Helper.qll | 2 +- .../security/ArtifactPoisoningQuery.qll | 3 ++- .../.github/workflows/artifactpoisoning93.yml | 19 +++++++++++++++++++ .../.github/workflows/artifactpoisoning94.yml | 19 +++++++++++++++++++ .../.github/workflows/artifactpoisoning95.yml | 19 +++++++++++++++++++ .../.github/workflows/artifactpoisoning96.yml | 18 ++++++++++++++++++ .../ArtifactPoisoningCritical.expected | 4 ++++ .../CWE-829/ArtifactPoisoningMedium.expected | 3 +++ .../UntrustedCheckoutCritical.expected | 8 ++++++++ 9 files changed, 93 insertions(+), 2 deletions(-) create mode 100644 actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning93.yml create mode 100644 actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning94.yml create mode 100644 actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning95.yml create mode 100644 actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning96.yml diff --git a/actions/ql/lib/codeql/actions/Helper.qll b/actions/ql/lib/codeql/actions/Helper.qll index 48b70061ec0f..348ec75a5193 100644 --- a/actions/ql/lib/codeql/actions/Helper.qll +++ b/actions/ql/lib/codeql/actions/Helper.qll @@ -72,7 +72,7 @@ string normalizePath(string path) { then result = path else // foo -> GITHUB_WORKSPACE/foo - if path.regexpMatch("^[^/~].*") + if path.regexpMatch("^[^$/~].*") then result = "GITHUB_WORKSPACE/" + path.regexpReplaceAll("/$", "") else // ~/foo -> ~/foo diff --git a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll index 0db3b2f7f887..778263f2ef25 100644 --- a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll +++ b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll @@ -262,9 +262,10 @@ class ArtifactPoisoningSink extends DataFlow::Node { ArtifactPoisoningSink() { download.getAFollowingStep() = poisonable and - // excluding artifacts downloaded to /tmp and runner.tmp + // excluding artifacts downloaded to the temporary directory not download.getPath().regexpMatch("^/tmp.*") and not download.getPath().regexpMatch("^\\$\\{\\{\\s?runner\\.temp\\s?}}.*") and + not download.getPath().regexpMatch("^\\$RUNNER_TEMP.*") and ( poisonable.(Run).getScript() = this.asExpr() and ( diff --git a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning93.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning93.yml new file mode 100644 index 000000000000..5bebc3484e68 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning93.yml @@ -0,0 +1,19 @@ +on: + workflow_run: + workflows: + - Benchmark + types: + - completed + +jobs: + benchmark: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Download From PR + uses: actions/download-artifact@v4 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + path: ${{ runner.temp }}/artifacts/ + - run: npm install \ No newline at end of file diff --git a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning94.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning94.yml new file mode 100644 index 000000000000..bee8abd41206 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning94.yml @@ -0,0 +1,19 @@ +on: + workflow_run: + workflows: + - Benchmark + types: + - completed + +jobs: + benchmark: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Download From PR + uses: actions/download-artifact@v4 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + path: /tmp/artifacts/ + - run: npm install \ No newline at end of file diff --git a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning95.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning95.yml new file mode 100644 index 000000000000..7a1aca467604 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning95.yml @@ -0,0 +1,19 @@ +on: + workflow_run: + workflows: + - Benchmark + types: + - completed + +jobs: + benchmark: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Download From PR + uses: actions/download-artifact@v4 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + path: $RUNNER_TEMP/artifacts/ + - run: npm install \ No newline at end of file diff --git a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning96.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning96.yml new file mode 100644 index 000000000000..f970a3fa1979 --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning96.yml @@ -0,0 +1,18 @@ +on: + workflow_run: + workflows: + - Benchmark + types: + - completed + +jobs: + benchmark: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Download From PR + uses: actions/download-artifact@v4 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + - run: npm install \ No newline at end of file diff --git a/actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningCritical.expected b/actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningCritical.expected index aa0057d60a1b..2d29cd9b79b4 100644 --- a/actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningCritical.expected +++ b/actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningCritical.expected @@ -13,6 +13,7 @@ edges | .github/workflows/artifactpoisoning42.yml:13:9:21:6 | Run Step | .github/workflows/artifactpoisoning42.yml:22:14:22:18 | ./cmd | provenance | Config | | .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 | | .github/workflows/artifactpoisoning81.yml:28:9:31:6 | Uses Step | .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | provenance | Config | +| .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | provenance | Config | | .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 | | .github/workflows/test18.yml:12:15:33:12 | Uses Step | .github/workflows/test18.yml:36:15:40:58 | Uses Step | provenance | Config | | .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 | .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | semmle.label | python test.py | | .github/workflows/artifactpoisoning92.yml:28:9:29:6 | Uses Step | semmle.label | Uses Step | | .github/workflows/artifactpoisoning92.yml:29:14:29:26 | make snapshot | semmle.label | make snapshot | +| .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | semmle.label | Uses Step | +| .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | semmle.label | npm install | | .github/workflows/artifactpoisoning101.yml:10:9:16:6 | Uses Step | semmle.label | 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 | semmle.label | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | | .github/workflows/test18.yml:12:15:33:12 | Uses Step | semmle.label | Uses Step | @@ -66,6 +69,7 @@ subpaths | .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 | | .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 | | .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 | +| .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 | | .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 | | .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 | | .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 | diff --git a/actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningMedium.expected b/actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningMedium.expected index 09aed9e34a10..c0c52e47f5b3 100644 --- a/actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningMedium.expected +++ b/actions/ql/test/query-tests/Security/CWE-829/ArtifactPoisoningMedium.expected @@ -13,6 +13,7 @@ edges | .github/workflows/artifactpoisoning42.yml:13:9:21:6 | Run Step | .github/workflows/artifactpoisoning42.yml:22:14:22:18 | ./cmd | provenance | Config | | .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 | | .github/workflows/artifactpoisoning81.yml:28:9:31:6 | Uses Step | .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | provenance | Config | +| .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | provenance | Config | | .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 | | .github/workflows/test18.yml:12:15:33:12 | Uses Step | .github/workflows/test18.yml:36:15:40:58 | Uses Step | provenance | Config | | .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 | .github/workflows/artifactpoisoning81.yml:31:14:31:27 | python test.py | semmle.label | python test.py | | .github/workflows/artifactpoisoning92.yml:28:9:29:6 | Uses Step | semmle.label | Uses Step | | .github/workflows/artifactpoisoning92.yml:29:14:29:26 | make snapshot | semmle.label | make snapshot | +| .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | semmle.label | Uses Step | +| .github/workflows/artifactpoisoning96.yml:18:14:18:24 | npm install | semmle.label | npm install | | .github/workflows/artifactpoisoning101.yml:10:9:16:6 | Uses Step | semmle.label | 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 | semmle.label | PR_NUMBER=$(./get_pull_request_number.sh pr_number.txt)\necho "PR_NUMBER=$PR_NUMBER" >> $GITHUB_OUTPUT \n | | .github/workflows/test18.yml:12:15:33:12 | Uses Step | semmle.label | Uses Step | diff --git a/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected b/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected index 5d7a94aead0b..9e54eb48c9aa 100644 --- a/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected +++ b/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected @@ -51,6 +51,14 @@ edges | .github/workflows/artifactpoisoning92.yml:19:9:25:6 | Run Step: metadata | .github/workflows/artifactpoisoning92.yml:25:9:28:6 | Uses Step | | .github/workflows/artifactpoisoning92.yml:25:9:28:6 | Uses Step | .github/workflows/artifactpoisoning92.yml:28:9:29:6 | Uses Step | | .github/workflows/artifactpoisoning92.yml:28:9:29:6 | Uses Step | .github/workflows/artifactpoisoning92.yml:29:9:29:27 | Run Step | +| .github/workflows/artifactpoisoning93.yml:12:9:13:6 | Uses Step | .github/workflows/artifactpoisoning93.yml:13:9:19:6 | Uses Step | +| .github/workflows/artifactpoisoning93.yml:13:9:19:6 | Uses Step | .github/workflows/artifactpoisoning93.yml:19:9:19:24 | Run Step | +| .github/workflows/artifactpoisoning94.yml:12:9:13:6 | Uses Step | .github/workflows/artifactpoisoning94.yml:13:9:19:6 | Uses Step | +| .github/workflows/artifactpoisoning94.yml:13:9:19:6 | Uses Step | .github/workflows/artifactpoisoning94.yml:19:9:19:24 | Run Step | +| .github/workflows/artifactpoisoning95.yml:12:9:13:6 | Uses Step | .github/workflows/artifactpoisoning95.yml:13:9:19:6 | Uses Step | +| .github/workflows/artifactpoisoning95.yml:13:9:19:6 | Uses Step | .github/workflows/artifactpoisoning95.yml:19:9:19:24 | Run Step | +| .github/workflows/artifactpoisoning96.yml:12:9:13:6 | Uses Step | .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | +| .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning96.yml:18:9:18:24 | Run Step | | .github/workflows/artifactpoisoning101.yml:10:9:16:6 | Uses Step | .github/workflows/artifactpoisoning101.yml:16:9:19:59 | Run Step: pr_number | | .github/workflows/auto_ci.yml:20:9:27:6 | Uses Step | .github/workflows/auto_ci.yml:27:9:32:6 | Uses Step | | .github/workflows/auto_ci.yml:27:9:32:6 | Uses Step | .github/workflows/auto_ci.yml:32:9:37:6 | Run Step | From e40e4c3856a9afd349e5b65c0efe367d3eb4d6b0 Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Wed, 9 Jul 2025 23:06:18 -0400 Subject: [PATCH 6/9] Remove unneeded test file. --- .../.github/workflows/artifactpoison93.yml | 22 ------------------- 1 file changed, 22 deletions(-) delete mode 100644 actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoison93.yml diff --git a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoison93.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoison93.yml deleted file mode 100644 index d463527f292c..000000000000 --- a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoison93.yml +++ /dev/null @@ -1,22 +0,0 @@ -name: Secure Workflow - -on: - workflow_run: - workflows: ["Prev"] - types: - - completed - -jobs: - Download: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - - run: mkdir -p ${{ runner.temp }}/artifacts/ - - uses: dawidd6/action-download-artifact@v2 - with: - name: pr_number - path: ${{ runner.temp }}/artifacts/ - - - name: Run command - run: | - sh cmd.sh From 7be938c6c390eec61400733b93b7d86e09773441 Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Thu, 10 Jul 2025 12:22:14 -0400 Subject: [PATCH 7/9] Handle multiple whitespaces in runner temp regex. Co-authored-by: Napalys Klicius --- .../ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll index 778263f2ef25..f0649bb8174e 100644 --- a/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll +++ b/actions/ql/lib/codeql/actions/security/ArtifactPoisoningQuery.qll @@ -264,7 +264,7 @@ class ArtifactPoisoningSink extends DataFlow::Node { download.getAFollowingStep() = poisonable and // excluding artifacts downloaded to the temporary directory not download.getPath().regexpMatch("^/tmp.*") and - not download.getPath().regexpMatch("^\\$\\{\\{\\s?runner\\.temp\\s?}}.*") and + not download.getPath().regexpMatch("^\\$\\{\\{\\s*runner\\.temp\\s*}}.*") and not download.getPath().regexpMatch("^\\$RUNNER_TEMP.*") and ( poisonable.(Run).getScript() = this.asExpr() and From 1b794e056af9947e6af39855e2040ed3bb1806f8 Mon Sep 17 00:00:00 2001 From: AdnaneKhan Date: Thu, 10 Jul 2025 12:24:36 -0400 Subject: [PATCH 8/9] Add extra test suggested by @Napalys --- .../.github/workflows/artifactpoisoning97.yml | 19 +++++++++++++++++++ 1 file changed, 19 insertions(+) create mode 100644 actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning97.yml diff --git a/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning97.yml b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning97.yml new file mode 100644 index 000000000000..1bd28bafdd6e --- /dev/null +++ b/actions/ql/test/query-tests/Security/CWE-829/.github/workflows/artifactpoisoning97.yml @@ -0,0 +1,19 @@ +on: + workflow_run: + workflows: + - Benchmark + types: + - completed + +jobs: + benchmark: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + - name: Download From PR + uses: actions/download-artifact@v4 + with: + github-token: ${{ secrets.GITHUB_TOKEN }} + run-id: ${{ github.event.workflow_run.id }} + path: ${{ runner.temp }}/artifacts/ + - run: npm install From 07598e8b62eac17ec34150c8b71326ae90335b90 Mon Sep 17 00:00:00 2001 From: Adnan Khan Date: Fri, 11 Jul 2025 05:59:13 +0000 Subject: [PATCH 9/9] Add test results. --- .../Security/CWE-829/UntrustedCheckoutCritical.expected | 2 ++ 1 file changed, 2 insertions(+) diff --git a/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected b/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected index 9e54eb48c9aa..39e54b2bbaed 100644 --- a/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected +++ b/actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected @@ -59,6 +59,8 @@ edges | .github/workflows/artifactpoisoning95.yml:13:9:19:6 | Uses Step | .github/workflows/artifactpoisoning95.yml:19:9:19:24 | Run Step | | .github/workflows/artifactpoisoning96.yml:12:9:13:6 | Uses Step | .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | | .github/workflows/artifactpoisoning96.yml:13:9:18:6 | Uses Step | .github/workflows/artifactpoisoning96.yml:18:9:18:24 | Run Step | +| .github/workflows/artifactpoisoning97.yml:12:9:13:6 | Uses Step | .github/workflows/artifactpoisoning97.yml:13:9:19:6 | Uses Step | +| .github/workflows/artifactpoisoning97.yml:13:9:19:6 | Uses Step | .github/workflows/artifactpoisoning97.yml:19:9:19:25 | Run Step | | .github/workflows/artifactpoisoning101.yml:10:9:16:6 | Uses Step | .github/workflows/artifactpoisoning101.yml:16:9:19:59 | Run Step: pr_number | | .github/workflows/auto_ci.yml:20:9:27:6 | Uses Step | .github/workflows/auto_ci.yml:27:9:32:6 | Uses Step | | .github/workflows/auto_ci.yml:27:9:32:6 | Uses Step | .github/workflows/auto_ci.yml:32:9:37:6 | Run Step |