From 0d3bb8919500d3cd5bf93d901c9288641aaa6474 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 23 Jun 2025 14:36:15 +0200 Subject: [PATCH 1/8] JS: Deprecate Actions.qll --- javascript/ql/lib/semmle/javascript/Actions.qll | 3 +++ 1 file changed, 3 insertions(+) diff --git a/javascript/ql/lib/semmle/javascript/Actions.qll b/javascript/ql/lib/semmle/javascript/Actions.qll index 8854eb11a55..d3f19681d88 100644 --- a/javascript/ql/lib/semmle/javascript/Actions.qll +++ b/javascript/ql/lib/semmle/javascript/Actions.qll @@ -1,7 +1,10 @@ /** + * DEPRECATED. Models for GitHub Actions workflow files are part of the actions qlpack now. + * * Libraries for modeling GitHub Actions workflow files written in YAML. * See https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions. */ +deprecated module; import javascript From 3a00e8d1c50ac63d0527d5e09324f3c21c6f6f57 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 23 Jun 2025 14:37:21 +0200 Subject: [PATCH 2/8] JS: Remove js/actions/pull-request-target Superseded by actions/untrusted-checkout/{medium,high,critical} --- .../Security/CWE-094/UntrustedCheckout.qhelp | 64 --------------- .../Security/CWE-094/UntrustedCheckout.ql | 81 ------------------- .../Security/CWE-094/examples/comment_pr.yml | 52 ------------ .../examples/pull_request_target_bad.yml | 25 ------ .../Security/CWE-094/examples/receive_pr.yml | 26 ------ .../workflows/pull_request_target_if_job.yml | 38 --------- .../workflows/pull_request_target_if_step.yml | 31 ------- .../pull_request_target_label_only.yml | 12 --- ...pull_request_target_label_only_mapping.yml | 13 --- .../pull_request_target_labels_mapping.yml | 15 ---- .../pull_request_target_labels_sequence.yml | 12 --- .../workflows/pull_request_target_mapping.yml | 10 --- .../workflows/pull_request_target_master.yml | 10 --- .../workflows/pull_request_target_run.yml | 11 --- .../pull_request_target_sequence.yml | 9 --- .../CWE-094/UntrustedCheckout.expected | 15 ---- .../Security/CWE-094/UntrustedCheckout.qlref | 1 - 17 files changed, 425 deletions(-) delete mode 100644 javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.qhelp delete mode 100644 javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.ql delete mode 100644 javascript/ql/src/experimental/Security/CWE-094/examples/comment_pr.yml delete mode 100644 javascript/ql/src/experimental/Security/CWE-094/examples/pull_request_target_bad.yml delete mode 100644 javascript/ql/src/experimental/Security/CWE-094/examples/receive_pr.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_if_job.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_if_step.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_label_only.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_label_only_mapping.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_labels_mapping.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_labels_sequence.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_mapping.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_master.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_run.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_sequence.yml delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/UntrustedCheckout.expected delete mode 100644 javascript/ql/test/experimental/Security/CWE-094/UntrustedCheckout.qlref diff --git a/javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.qhelp b/javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.qhelp deleted file mode 100644 index 4833b50d8e2..00000000000 --- a/javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.qhelp +++ /dev/null @@ -1,64 +0,0 @@ - - - - - -

- - Combining pull_request_target workflow trigger with an explicit checkout - of an untrusted pull request is a dangerous practice - that may lead to repository compromise. - -

- -
- - - -

- - The best practice is to handle the potentially untrusted pull request - via the pull_request trigger so that it is isolated in - an unprivileged environment. The workflow processing the pull request - should then store any results like code coverage or failed/passed tests - in artifacts and exit. The following workflow then starts on workflow_run - where it is granted write permission to the target repository and access to - repository secrets, so that it can download the artifacts and make - any necessary modifications to the repository or interact with third party services - that require repository secrets (e.g. API tokens). - -

- -
- - - -

- - The following example allows unauthorized repository modification - and secrets exfiltration: - -

- - - -

- - The following example uses two workflows to handle potentially untrusted - pull request in a secure manner. The receive_pr.yml is triggered first: - -

- - -

The comment_pr.yml is triggered after receive_pr.yml completes:

- - -
- - -
  • GitHub Security Lab Research: Keeping your GitHub Actions and workflows secure: Preventing pwn requests.
  • -
    - -
    diff --git a/javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.ql b/javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.ql deleted file mode 100644 index 3f08f297c6a..00000000000 --- a/javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.ql +++ /dev/null @@ -1,81 +0,0 @@ -/** - * @name Checkout of untrusted code in trusted context - * @description Workflows triggered on `pull_request_target` have read/write access to the base repository and access to secrets. - * By explicitly checking out and running the build script from a fork the untrusted code is running in an environment - * that is able to push to the base repository and to access secrets. - * @kind problem - * @problem.severity warning - * @precision low - * @id js/actions/pull-request-target - * @tags actions - * security - * experimental - * external/cwe/cwe-094 - */ - -import javascript -import semmle.javascript.Actions - -/** - * An action step that doesn't contain `actor` check in `if:` or - * the check requires manual analysis. - */ -class ProbableStep extends Actions::Step { - // some simplistic checks to eleminate likely false positives: - ProbableStep() { - // no if at all - not exists(this.getIf().getValue()) - or - // needs manual analysis if there is OR - this.getIf().getValue().matches("%||%") - or - // actor check means only the user is able to run it - not exists(this.getIf().getValue().regexpFind("\\bgithub\\s*\\.\\s*actor\\s*==", _, _)) - } -} - -/** - * An action job that doesn't contain `actor` check in `if:` or - * the check requires manual analysis. - */ -class ProbableJob extends Actions::Job { - // some simplistic checks to eleminate likely false positives: - ProbableJob() { - // no if at all - not exists(this.getIf().getValue()) - or - // needs manual analysis if there is OR - this.getIf().getValue().matches("%||%") - or - // actor check means only the user is able to run it - not exists(this.getIf().getValue().regexpFind("\\bgithub\\s*\\.\\s*actor\\s*==", _, _)) - } -} - -/** - * The `on: pull_request_target`. - */ -class ProbablePullRequestTarget extends Actions::On, YamlMappingLikeNode { - ProbablePullRequestTarget() { - // The `on:` is triggered on `pull_request_target` - exists(this.getNode("pull_request_target")) - } -} - -from - Actions::Ref ref, Actions::Uses uses, Actions::Step step, Actions::Job job, - ProbablePullRequestTarget pullRequestTarget -where - pullRequestTarget.getWorkflow() = job.getWorkflow() and - uses.getStep() = step and - ref.getWith().getStep() = step and - step.getJob() = job and - uses.getGitHubRepository() = "actions/checkout" and - ref.getValue() - .matches([ - "%github.event.pull_request.head.ref%", "%github.event.pull_request.head.sha%", - "%github.event.pull_request.number%", "%github.event.number%", "%github.head_ref%" - ]) and - step instanceof ProbableStep and - job instanceof ProbableJob -select step, "Potential unsafe checkout of untrusted pull request on 'pull_request_target'." diff --git a/javascript/ql/src/experimental/Security/CWE-094/examples/comment_pr.yml b/javascript/ql/src/experimental/Security/CWE-094/examples/comment_pr.yml deleted file mode 100644 index e496b1449a0..00000000000 --- a/javascript/ql/src/experimental/Security/CWE-094/examples/comment_pr.yml +++ /dev/null @@ -1,52 +0,0 @@ -name: Comment on the pull request - -# read-write repo token -# access to secrets -on: - workflow_run: - workflows: ["Receive PR"] - types: - - completed - -jobs: - upload: - runs-on: ubuntu-latest - if: > - ${{ github.event.workflow_run.event == 'pull_request' && - github.event.workflow_run.conclusion == 'success' }} - steps: - - name: 'Download artifact' - uses: actions/github-script@v3.1.0 - with: - script: | - var artifacts = await github.actions.listWorkflowRunArtifacts({ - owner: context.repo.owner, - repo: context.repo.repo, - run_id: ${{github.event.workflow_run.id }}, - }); - var matchArtifact = artifacts.data.artifacts.filter((artifact) => { - return artifact.name == "pr" - })[0]; - var download = await github.actions.downloadArtifact({ - owner: context.repo.owner, - repo: context.repo.repo, - artifact_id: matchArtifact.id, - archive_format: 'zip', - }); - var fs = require('fs'); - fs.writeFileSync('${{github.workspace}}/pr.zip', Buffer.from(download.data)); - - run: unzip pr.zip - - - name: 'Comment on PR' - uses: actions/github-script@v3 - with: - github-token: ${{ secrets.GITHUB_TOKEN }} - script: | - var fs = require('fs'); - var issue_number = Number(fs.readFileSync('./NR')); - await github.issues.createComment({ - owner: context.repo.owner, - repo: context.repo.repo, - issue_number: issue_number, - body: 'Everything is OK. Thank you for the PR!' - }); \ No newline at end of file diff --git a/javascript/ql/src/experimental/Security/CWE-094/examples/pull_request_target_bad.yml b/javascript/ql/src/experimental/Security/CWE-094/examples/pull_request_target_bad.yml deleted file mode 100644 index fb9f0699a42..00000000000 --- a/javascript/ql/src/experimental/Security/CWE-094/examples/pull_request_target_bad.yml +++ /dev/null @@ -1,25 +0,0 @@ -on: - pull_request_target - -jobs: - build: - name: Build and test - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.sha }} - - - uses: actions/setup-node@v1 - - run: | - npm install - npm build - - - uses: completely/fakeaction@v2 - with: - arg1: ${{ secrets.supersecret }} - - - uses: fakerepo/comment-on-pr@v1 - with: - message: | - Thank you! \ No newline at end of file diff --git a/javascript/ql/src/experimental/Security/CWE-094/examples/receive_pr.yml b/javascript/ql/src/experimental/Security/CWE-094/examples/receive_pr.yml deleted file mode 100644 index 7104bce8bf3..00000000000 --- a/javascript/ql/src/experimental/Security/CWE-094/examples/receive_pr.yml +++ /dev/null @@ -1,26 +0,0 @@ -name: Receive PR - -# read-only repo token -# no access to secrets -on: - pull_request: - -jobs: - build: - runs-on: ubuntu-latest - - steps: - - uses: actions/checkout@v2 - - # imitation of a build process - - name: Build - run: /bin/bash ./build.sh - - - name: Save PR number - run: | - mkdir -p ./pr - echo ${{ github.event.number }} > ./pr/NR - - uses: actions/upload-artifact@v2 - with: - name: pr - path: pr/ \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_if_job.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_if_job.yml deleted file mode 100644 index cf713f5473d..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_if_job.yml +++ /dev/null @@ -1,38 +0,0 @@ -on: - pull_request_target: - -jobs: - job1: - if: contains(github.event.issue.labels.*.name, 'ok') - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} - job2: - if: github.event.label.name == 'ok' - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} - job3: - if: github.actor == 'ok' - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} - job4: - if: github.actor == 'ok' || true - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} - job5: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_if_step.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_if_step.yml deleted file mode 100644 index 08a3757392b..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_if_step.yml +++ /dev/null @@ -1,31 +0,0 @@ -on: - pull_request_target: - -jobs: - job1: - runs-on: ubuntu-latest - steps: - - - uses: actions/checkout@v2 - if: contains(github.event.issue.labels.*.name, 'ok') - with: - ref: ${{ github.event.pull_request.head.ref }} - - - uses: actions/checkout@v2 - if: github.event.label.name == 'ok' - with: - ref: ${{ github.event.pull_request.head.ref }} - - - uses: actions/checkout@v2 - if: github.actor == 'ok' - with: - ref: ${{ github.event.pull_request.head.ref }} - - - uses: actions/checkout@v2 - if: github.actor == 'ok' || true - with: - ref: ${{ github.event.pull_request.head.ref }} - - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_label_only.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_label_only.yml deleted file mode 100644 index 1b87798b89c..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_label_only.yml +++ /dev/null @@ -1,12 +0,0 @@ -on: - pull_request_target: - types: [labeled] - push: - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_label_only_mapping.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_label_only_mapping.yml deleted file mode 100644 index 076d827a9b9..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_label_only_mapping.yml +++ /dev/null @@ -1,13 +0,0 @@ -on: - pull_request_target: - types: - labeled: - push: - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_labels_mapping.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_labels_mapping.yml deleted file mode 100644 index 5b2b594890b..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_labels_mapping.yml +++ /dev/null @@ -1,15 +0,0 @@ -on: - pull_request_target: - types: - labeled: - opened: - closed: - push: - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_labels_sequence.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_labels_sequence.yml deleted file mode 100644 index 9677bd35360..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_labels_sequence.yml +++ /dev/null @@ -1,12 +0,0 @@ -on: - pull_request_target: - types: [labeled, opened] - push: - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_mapping.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_mapping.yml deleted file mode 100644 index c3875fde7cd..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_mapping.yml +++ /dev/null @@ -1,10 +0,0 @@ -on: - pull_request_target: - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_master.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_master.yml deleted file mode 100644 index feeec7bf1f3..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_master.yml +++ /dev/null @@ -1,10 +0,0 @@ -on: - pull_request_target: - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: master \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_run.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_run.yml deleted file mode 100644 index 33bb9ba889b..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_run.yml +++ /dev/null @@ -1,11 +0,0 @@ -on: pull_request_target - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} - - - run: make \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_sequence.yml b/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_sequence.yml deleted file mode 100644 index 5e0d2a8ee27..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/.github/workflows/pull_request_target_sequence.yml +++ /dev/null @@ -1,9 +0,0 @@ -on: [pull_request_target, push] - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v2 - with: - ref: ${{ github.event.pull_request.head.ref }} \ No newline at end of file diff --git a/javascript/ql/test/experimental/Security/CWE-094/UntrustedCheckout.expected b/javascript/ql/test/experimental/Security/CWE-094/UntrustedCheckout.expected deleted file mode 100644 index 127ced2bb97..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/UntrustedCheckout.expected +++ /dev/null @@ -1,15 +0,0 @@ -| .github/workflows/pull_request_target_if_job.yml:9:7:12:2 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_if_job.yml:16:7:19:2 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_if_job.yml:30:7:33:2 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_if_job.yml:36:7:38:54 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_if_step.yml:9:7:14:4 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_if_step.yml:14:7:19:4 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_if_step.yml:24:7:29:4 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_if_step.yml:29:7:31:54 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_label_only.yml:10:7:12:54 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_label_only_mapping.yml:11:7:13:54 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_labels_mapping.yml:13:7:15:54 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_labels_sequence.yml:10:7:12:54 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_mapping.yml:8:7:10:54 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_run.yml:7:7:11:4 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | -| .github/workflows/pull_request_target_sequence.yml:7:7:9:54 | uses: a ... kout@v2 | Potential unsafe checkout of untrusted pull request on 'pull_request_target'. | diff --git a/javascript/ql/test/experimental/Security/CWE-094/UntrustedCheckout.qlref b/javascript/ql/test/experimental/Security/CWE-094/UntrustedCheckout.qlref deleted file mode 100644 index bdf753c1f4a..00000000000 --- a/javascript/ql/test/experimental/Security/CWE-094/UntrustedCheckout.qlref +++ /dev/null @@ -1 +0,0 @@ -experimental/Security/CWE-094/UntrustedCheckout.ql From 9dcb61e771814a03d3a072cbc92f7fbe04615838 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 23 Jun 2025 14:39:19 +0200 Subject: [PATCH 3/8] JS: Remove js/actions/actions-artifact-leak Superseded by actions/secrets-in-artifacts --- .../CWE-312/ActionsArtifactLeak.qhelp | 30 ----- .../Security/CWE-312/ActionsArtifactLeak.ql | 112 ------------------ .../examples/actions-artifact-leak-fixed.yml | 14 --- .../examples/actions-artifact-leak.yml | 13 -- .../CWE-312/.github/workflows/test.yml | 87 -------------- .../CWE-312/ActionsArtifactLeak.expected | 5 - .../CWE-312/ActionsArtifactLeak.qlref | 1 - 7 files changed, 262 deletions(-) delete mode 100644 javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.qhelp delete mode 100644 javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.ql delete mode 100644 javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak-fixed.yml delete mode 100644 javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-312/.github/workflows/test.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.expected delete mode 100644 javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.qlref diff --git a/javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.qhelp b/javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.qhelp deleted file mode 100644 index 7ec9c1fe777..00000000000 --- a/javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.qhelp +++ /dev/null @@ -1,30 +0,0 @@ - - - -

    - Sensitive information included in a GitHub Actions artifact can allow an attacker to access - the sensitive information if the artifact is published. -

    -
    - - -

    - Only store information that is meant to be publicly available in a GitHub Actions artifact. -

    -
    - - -

    - The following example uses actions/checkout to checkout code which stores the GITHUB_TOKEN in the `.git/config` file - and then stores the contents of the `.git` repository into the artifact: -

    - -

    - The issue has been fixed below, where the actions/upload-artifact uses a version (v4+) which does not include hidden files or - directories into the artifact. -

    - -
    -
    diff --git a/javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.ql b/javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.ql deleted file mode 100644 index 3f001c5e456..00000000000 --- a/javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.ql +++ /dev/null @@ -1,112 +0,0 @@ -/** - * @name Storage of sensitive information in GitHub Actions artifact - * @description Including sensitive information in a GitHub Actions artifact can - * expose it to an attacker. - * @kind problem - * @problem.severity error - * @security-severity 7.5 - * @precision high - * @id js/actions/actions-artifact-leak - * @tags actions - * security - * external/cwe/cwe-312 - * external/cwe/cwe-315 - * external/cwe/cwe-359 - */ - -import javascript -import semmle.javascript.Actions - -/** - * A step that uses `actions/checkout` action. - */ -class ActionsCheckoutStep extends Actions::Step { - ActionsCheckoutStep() { this.getUses().getGitHubRepository() = "actions/checkout" } -} - -/** - * A `with:`/`persist-credentials` field sibling to `uses: actions/checkout`. - */ -class ActionsCheckoutWithPersistCredentials extends YamlNode, YamlScalar { - ActionsCheckoutStep step; - - ActionsCheckoutWithPersistCredentials() { - step.lookup("with").(YamlMapping).lookup("persist-credentials") = this - } - - /** Gets the step this field belongs to. */ - ActionsCheckoutStep getStep() { result = step } -} - -/** - * A `with:`/`path` field sibling to `uses: actions/checkout`. - */ -class ActionsCheckoutWithPath extends YamlNode, YamlString { - ActionsCheckoutStep step; - - ActionsCheckoutWithPath() { step.lookup("with").(YamlMapping).lookup("path") = this } - - /** Gets the step this field belongs to. */ - ActionsCheckoutStep getStep() { result = step } -} - -/** - * A step that uses `actions/upload-artifact` action. - */ -class ActionsUploadArtifactStep extends Actions::Step { - ActionsUploadArtifactStep() { this.getUses().getGitHubRepository() = "actions/upload-artifact" } -} - -/** - * A `with:`/`path` field sibling to `uses: actions/upload-artifact`. - */ -class ActionsUploadArtifactWithPath extends YamlNode, YamlString { - ActionsUploadArtifactStep step; - - ActionsUploadArtifactWithPath() { step.lookup("with").(YamlMapping).lookup("path") = this } - - /** Gets the step this field belongs to. */ - ActionsUploadArtifactStep getStep() { result = step } -} - -from ActionsCheckoutStep checkout, ActionsUploadArtifactStep upload, Actions::Job job, int i, int j -where - checkout.getJob() = job and - upload.getJob() = job and - job.getStep(i) = checkout and - job.getStep(j) = upload and - j = i + 1 and - upload.getUses().getVersion() = - [ - "v4.3.6", "834a144ee995460fba8ed112a2fc961b36a5ec5a", // - "v4.3.5", "89ef406dd8d7e03cfd12d9e0a4a378f454709029", // - "v4.3.4", "0b2256b8c012f0828dc542b3febcab082c67f72b", // - "v4.3.3", "65462800fd760344b1a7b4382951275a0abb4808", // - "v4.3.2", "1746f4ab65b179e0ea60a494b83293b640dd5bba", // - "v4.3.1", "5d5d22a31266ced268874388b861e4b58bb5c2f3", // - "v4.3.0", "26f96dfa697d77e81fd5907df203aa23a56210a8", // - "v4.2.0", "694cdabd8bdb0f10b2cea11669e1bf5453eed0a6", // - "v4.1.0", "1eb3cb2b3e0f29609092a73eb033bb759a334595", // - "v4.0.0", "c7d193f32edcb7bfad88892161225aeda64e9392", // - ] and - ( - not exists(ActionsCheckoutWithPersistCredentials persist | persist.getStep() = checkout) - or - exists(ActionsCheckoutWithPersistCredentials persist | - persist.getStep() = checkout and - persist.getValue() = "true" - ) - ) and - ( - not exists(ActionsCheckoutWithPath path | path.getStep() = checkout) and - exists(ActionsUploadArtifactWithPath path | - path.getStep() = upload and path.getValue() = [".", "*"] - ) - or - exists(ActionsCheckoutWithPath checkout_path, ActionsUploadArtifactWithPath upload_path | - checkout_path.getValue() + ["", "/*"] = upload_path.getValue() and - checkout_path.getStep() = checkout and - upload_path.getStep() = upload - ) - ) -select upload, "A secret may be exposed in an artifact." diff --git a/javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak-fixed.yml b/javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak-fixed.yml deleted file mode 100644 index 9f716584a9b..00000000000 --- a/javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak-fixed.yml +++ /dev/null @@ -1,14 +0,0 @@ -name: secrets-in-artifacts -on: - pull_request: -jobs: - a-job: # NOT VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: "Upload artifact" - uses: actions/upload-artifact@v4 - with: - name: file - path: . - diff --git a/javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak.yml b/javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak.yml deleted file mode 100644 index 9006855e997..00000000000 --- a/javascript/ql/src/Security/CWE-312/examples/actions-artifact-leak.yml +++ /dev/null @@ -1,13 +0,0 @@ -name: secrets-in-artifacts -on: - pull_request: -jobs: - a-job: # VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: "Upload artifact" - uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2 - with: - name: file - path: . diff --git a/javascript/ql/test/query-tests/Security/CWE-312/.github/workflows/test.yml b/javascript/ql/test/query-tests/Security/CWE-312/.github/workflows/test.yml deleted file mode 100644 index 473d5998695..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-312/.github/workflows/test.yml +++ /dev/null @@ -1,87 +0,0 @@ -name: secrets-in-artifacts -on: - pull_request: -jobs: - test1: # VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: "Upload artifact" - uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2 - with: - name: file - path: . - test2: # NOT VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: "Upload artifact" - uses: actions/upload-artifact@v4 - with: - name: file - path: . - test3: # VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - - name: "Upload artifact" - uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2 - with: - name: file - path: "*" - test4: # VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - path: foo - - name: "Upload artifact" - uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2 - with: - name: file - path: foo - test5: # VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - path: foo - - name: "Upload artifact" - uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2 - with: - name: file - path: foo/* - test6: # NOT VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - path: pr - - name: "Upload artifact" - uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2 - with: - name: file - path: foo - test7: # NOT VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: false - - name: "Upload artifact" - uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2 - with: - name: file - path: . - test8: # VULNERABLE - runs-on: ubuntu-latest - steps: - - uses: actions/checkout@v4 - with: - persist-credentials: true - - name: "Upload artifact" - uses: actions/upload-artifact@1746f4ab65b179e0ea60a494b83293b640dd5bba # v4.3.2 - with: - name: file - path: . - diff --git a/javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.expected b/javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.expected deleted file mode 100644 index 575ddda89a4..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.expected +++ /dev/null @@ -1,5 +0,0 @@ -| .github/workflows/test.yml:9:9:14:2 | name: " ... tifact" | A secret may be exposed in an artifact. | -| .github/workflows/test.yml:27:9:32:2 | name: " ... tifact" | A secret may be exposed in an artifact. | -| .github/workflows/test.yml:38:9:43:2 | name: " ... tifact" | A secret may be exposed in an artifact. | -| .github/workflows/test.yml:49:9:54:2 | name: " ... tifact" | A secret may be exposed in an artifact. | -| .github/workflows/test.yml:82:9:86:18 | name: " ... tifact" | A secret may be exposed in an artifact. | diff --git a/javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.qlref b/javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.qlref deleted file mode 100644 index 79d8f4aea27..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-312/ActionsArtifactLeak.qlref +++ /dev/null @@ -1 +0,0 @@ -query: Security/CWE-312/ActionsArtifactLeak.ql From 76b7228160a5e686c1311a720cddfbc224824090 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 23 Jun 2025 14:41:26 +0200 Subject: [PATCH 4/8] JS: Remove js/actions/command-injection Superseded by actions/command-injection/{medium,critical} --- .../CWE-094/ExpressionInjection.qhelp | 56 ---- .../Security/CWE-094/ExpressionInjection.ql | 270 ------------------ .../CWE-094/examples/comment_issue_bad.yml | 8 - .../examples/comment_issue_bad_env.yml | 10 - .../CWE-094/examples/comment_issue_good.yml | 10 - .../.github/workflows/comment_issue.yml | 28 -- .../workflows/comment_issue_newline.yml | 10 - .../.github/workflows/discussion.yml | 8 - .../.github/workflows/discussion_comment.yml | 9 - .../.github/workflows/gollum.yml | 11 - .../.github/workflows/issues.yaml | 20 -- .../.github/workflows/pull_request_review.yml | 14 - .../workflows/pull_request_review_comment.yml | 14 - .../.github/workflows/pull_request_target.yml | 16 -- .../.github/workflows/push.yml | 16 -- .../.github/workflows/workflow_run.yml | 16 -- .../ExpressionInjection.expected | 65 ----- .../ExpressionInjection.qlref | 1 - .../ExpressionInjection/action1/action.yml | 14 - .../ExpressionInjection/action2/action.yml | 17 -- 20 files changed, 613 deletions(-) delete mode 100644 javascript/ql/src/Security/CWE-094/ExpressionInjection.qhelp delete mode 100644 javascript/ql/src/Security/CWE-094/ExpressionInjection.ql delete mode 100644 javascript/ql/src/Security/CWE-094/examples/comment_issue_bad.yml delete mode 100644 javascript/ql/src/Security/CWE-094/examples/comment_issue_bad_env.yml delete mode 100644 javascript/ql/src/Security/CWE-094/examples/comment_issue_good.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/comment_issue.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/comment_issue_newline.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/discussion.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/discussion_comment.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/gollum.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/issues.yaml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_review.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_review_comment.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_target.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/push.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/workflow_run.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.expected delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.qlref delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/action1/action.yml delete mode 100644 javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/action2/action.yml diff --git a/javascript/ql/src/Security/CWE-094/ExpressionInjection.qhelp b/javascript/ql/src/Security/CWE-094/ExpressionInjection.qhelp deleted file mode 100644 index df9d97e4e6b..00000000000 --- a/javascript/ql/src/Security/CWE-094/ExpressionInjection.qhelp +++ /dev/null @@ -1,56 +0,0 @@ - - - -

    - Using user-controlled input in GitHub Actions may lead to - code injection in contexts like run: or script:. -

    -

    - Code injection in GitHub Actions may allow an attacker to - exfiltrate any secrets used in the workflow and - the temporary GitHub repository authorization token. - The token might have write access to the repository, allowing an attacker - to use the token to make changes to the repository. -

    -
    - - -

    - The best practice to avoid code injection vulnerabilities - in GitHub workflows is to set the untrusted input value of the expression - to an intermediate environment variable and then use the environment variable - using the native syntax of the shell/script interpreter (that is, not ${{ env.VAR }}). -

    -

    - It is also recommended to limit the permissions of any tokens used - by a workflow such as the GITHUB_TOKEN. -

    -
    - - -

    - The following example lets a user inject an arbitrary shell command: -

    - - -

    - The following example uses an environment variable, but - still allows the injection because of the use of expression syntax: -

    - - -

    - The following example uses shell syntax to read - the environment variable and will prevent the attack: -

    - -
    - - -
  • GitHub Security Lab Research: Keeping your GitHub Actions and workflows secure: Untrusted input.
  • -
  • GitHub Docs: Security hardening for GitHub Actions.
  • -
  • GitHub Docs: Permissions for the GITHUB_TOKEN.
  • -
    -
    diff --git a/javascript/ql/src/Security/CWE-094/ExpressionInjection.ql b/javascript/ql/src/Security/CWE-094/ExpressionInjection.ql deleted file mode 100644 index 6c01edb330f..00000000000 --- a/javascript/ql/src/Security/CWE-094/ExpressionInjection.ql +++ /dev/null @@ -1,270 +0,0 @@ -/** - * @name Expression injection in Actions - * @description Using user-controlled GitHub Actions contexts like `run:` or `script:` may allow a malicious - * user to inject code into the GitHub action. - * @kind problem - * @problem.severity warning - * @security-severity 9.3 - * @precision high - * @id js/actions/command-injection - * @tags actions - * security - * external/cwe/cwe-094 - */ - -import javascript -import semmle.javascript.Actions - -/** - * A `script:` field within an Actions `with:` specific to `actions/github-script` action. - * - * For example: - * ``` - * uses: actions/github-script@v3 - * with: - * script: console.log('${{ github.event.pull_request.head.sha }}') - * ``` - */ -class GitHubScript extends YamlNode, YamlString { - GitHubScriptWith with; - - GitHubScript() { with.lookup("script") = this } - - /** Gets the `with` field this field belongs to. */ - GitHubScriptWith getWith() { result = with } -} - -/** - * A step that uses `actions/github-script` action. - */ -class GitHubScriptStep extends Actions::Step { - GitHubScriptStep() { this.getUses().getGitHubRepository() = "actions/github-script" } -} - -/** - * A `with:` field sibling to `uses: actions/github-script`. - */ -class GitHubScriptWith extends YamlNode, YamlMapping { - GitHubScriptStep step; - - GitHubScriptWith() { step.lookup("with") = this } - - /** Gets the step this field belongs to. */ - GitHubScriptStep getStep() { result = step } -} - -bindingset[context] -private predicate isExternalUserControlledIssue(string context) { - context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*issue\\s*\\.\\s*title\\b") or - context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*issue\\s*\\.\\s*body\\b") -} - -bindingset[context] -private predicate isExternalUserControlledPullRequest(string context) { - exists(string reg | - reg = - [ - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*title\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*body\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*head\\s*\\.\\s*label\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*head\\s*\\.\\s*repo\\s*\\.\\s*default_branch\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*head\\s*\\.\\s*repo\\s*\\.\\s*description\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*head\\s*\\.\\s*repo\\s*\\.\\s*homepage\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pull_request\\s*\\.\\s*head\\s*\\.\\s*ref\\b", - "\\bgithub\\s*\\.\\s*head_ref\\b" - ] - | - context.regexpMatch(reg) - ) -} - -bindingset[context] -private predicate isExternalUserControlledReview(string context) { - context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*review\\s*\\.\\s*body\\b") -} - -bindingset[context] -private predicate isExternalUserControlledComment(string context) { - context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*comment\\s*\\.\\s*body\\b") -} - -bindingset[context] -private predicate isExternalUserControlledGollum(string context) { - context - .regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pages\\[[0-9]+\\]\\s*\\.\\s*page_name\\b") or - context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*pages\\[[0-9]+\\]\\s*\\.\\s*title\\b") -} - -bindingset[context] -private predicate isExternalUserControlledCommit(string context) { - exists(string reg | - reg = - [ - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*commits\\[[0-9]+\\]\\s*\\.\\s*message\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*head_commit\\s*\\.\\s*message\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*head_commit\\s*\\.\\s*author\\s*\\.\\s*email\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*head_commit\\s*\\.\\s*author\\s*\\.\\s*name\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*head_commit\\s*\\.\\s*committer\\s*\\.\\s*email\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*head_commit\\s*\\.\\s*committer\\s*\\.\\s*name\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*commits\\[[0-9]+\\]\\s*\\.\\s*author\\s*\\.\\s*email\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*commits\\[[0-9]+\\]\\s*\\.\\s*author\\s*\\.\\s*name\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*commits\\[[0-9]+\\]\\s*\\.\\s*committer\\s*\\.\\s*email\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*commits\\[[0-9]+\\]\\s*\\.\\s*committer\\s*\\.\\s*name\\b", - ] - | - context.regexpMatch(reg) - ) -} - -bindingset[context] -private predicate isExternalUserControlledDiscussion(string context) { - context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*discussion\\s*\\.\\s*title\\b") or - context.regexpMatch("\\bgithub\\s*\\.\\s*event\\s*\\.\\s*discussion\\s*\\.\\s*body\\b") -} - -bindingset[context] -private predicate isExternalUserControlledWorkflowRun(string context) { - exists(string reg | - reg = - [ - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*workflow_run\\s*\\.\\s*head_branch\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*workflow_run\\s*\\.\\s*display_title\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*workflow_run\\s*\\.\\s*head_repository\\b\\s*\\.\\s*description\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*workflow_run\\s*\\.\\s*head_commit\\b\\s*\\.\\s*message\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*workflow_run\\s*\\.\\s*head_commit\\b\\s*\\.\\s*author\\b\\s*\\.\\s*email\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*workflow_run\\s*\\.\\s*head_commit\\b\\s*\\.\\s*author\\b\\s*\\.\\s*name\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*workflow_run\\s*\\.\\s*head_commit\\b\\s*\\.\\s*committer\\b\\s*\\.\\s*email\\b", - "\\bgithub\\s*\\.\\s*event\\s*\\.\\s*workflow_run\\s*\\.\\s*head_commit\\b\\s*\\.\\s*committer\\b\\s*\\.\\s*name\\b", - ] - | - context.regexpMatch(reg) - ) -} - -/** - * Holds if environment name in the `injection` (in a form of `env.name`) - * is tainted by the `context` (in a form of `github.event.xxx.xxx`). - */ -bindingset[injection] -predicate isEnvInterpolationTainted(string injection, string context) { - exists(Actions::Env env, string envName, YamlString envValue | - envValue = env.lookup(envName) and - Actions::getEnvName(injection) = envName and - Actions::getASimpleReferenceExpression(envValue) = context - ) -} - -/** - * Holds if the `run` contains any expression interpolation `${{ e }}`. - * Sets `context` to the initial untrusted value assignment in case of `${{ env... }}` interpolation - */ -predicate isRunInjectable(Actions::Run run, string injection, string context) { - Actions::getASimpleReferenceExpression(run) = injection and - ( - injection = context - or - isEnvInterpolationTainted(injection, context) - ) -} - -/** - * Holds if the `actions/github-script` contains any expression interpolation `${{ e }}`. - * Sets `context` to the initial untrusted value assignment in case of `${{ env... }}` interpolation - */ -predicate isScriptInjectable(GitHubScript script, string injection, string context) { - Actions::getASimpleReferenceExpression(script) = injection and - ( - injection = context - or - isEnvInterpolationTainted(injection, context) - ) -} - -/** - * Holds if the composite action contains untrusted expression interpolation `${{ e }}`. - */ -YamlNode getInjectableCompositeActionNode(Actions::Runs runs, string injection, string context) { - exists(Actions::Run run | - isRunInjectable(run, injection, context) and - result = run and - run.getStep().getRuns() = runs - ) - or - exists(GitHubScript script | - isScriptInjectable(script, injection, context) and - result = script and - script.getWith().getStep().getRuns() = runs - ) -} - -/** - * Holds if the workflow contains untrusted expression interpolation `${{ e }}`. - */ -YamlNode getInjectableWorkflowNode(Actions::On on, string injection, string context) { - exists(Actions::Run run | - isRunInjectable(run, injection, context) and - result = run and - run.getStep().getJob().getWorkflow().getOn() = on - ) - or - exists(GitHubScript script | - isScriptInjectable(script, injection, context) and - result = script and - script.getWith().getStep().getJob().getWorkflow().getOn() = on - ) -} - -from YamlNode node, string injection, string context -where - exists(Actions::CompositeAction action, Actions::Runs runs | - action.getRuns() = runs and - node = getInjectableCompositeActionNode(runs, injection, context) and - ( - isExternalUserControlledIssue(context) or - isExternalUserControlledPullRequest(context) or - isExternalUserControlledReview(context) or - isExternalUserControlledComment(context) or - isExternalUserControlledGollum(context) or - isExternalUserControlledCommit(context) or - isExternalUserControlledDiscussion(context) or - isExternalUserControlledWorkflowRun(context) - ) - ) - or - exists(Actions::On on | - node = getInjectableWorkflowNode(on, injection, context) and - ( - exists(on.getNode("issues")) and - isExternalUserControlledIssue(context) - or - exists(on.getNode("pull_request_target")) and - isExternalUserControlledPullRequest(context) - or - exists(on.getNode("pull_request_review")) and - (isExternalUserControlledReview(context) or isExternalUserControlledPullRequest(context)) - or - exists(on.getNode("pull_request_review_comment")) and - (isExternalUserControlledComment(context) or isExternalUserControlledPullRequest(context)) - or - exists(on.getNode("issue_comment")) and - (isExternalUserControlledComment(context) or isExternalUserControlledIssue(context)) - or - exists(on.getNode("gollum")) and - isExternalUserControlledGollum(context) - or - exists(on.getNode("push")) and - isExternalUserControlledCommit(context) - or - exists(on.getNode("discussion")) and - isExternalUserControlledDiscussion(context) - or - exists(on.getNode("discussion_comment")) and - (isExternalUserControlledDiscussion(context) or isExternalUserControlledComment(context)) - or - exists(on.getNode("workflow_run")) and - isExternalUserControlledWorkflowRun(context) - ) - ) -select node, - "Potential injection from the ${{ " + injection + - " }}, which may be controlled by an external user." diff --git a/javascript/ql/src/Security/CWE-094/examples/comment_issue_bad.yml b/javascript/ql/src/Security/CWE-094/examples/comment_issue_bad.yml deleted file mode 100644 index 1a25d44693b..00000000000 --- a/javascript/ql/src/Security/CWE-094/examples/comment_issue_bad.yml +++ /dev/null @@ -1,8 +0,0 @@ -on: issue_comment - -jobs: - echo-body: - runs-on: ubuntu-latest - steps: - - run: | - echo '${{ github.event.comment.body }}' \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-094/examples/comment_issue_bad_env.yml b/javascript/ql/src/Security/CWE-094/examples/comment_issue_bad_env.yml deleted file mode 100644 index b7698938de7..00000000000 --- a/javascript/ql/src/Security/CWE-094/examples/comment_issue_bad_env.yml +++ /dev/null @@ -1,10 +0,0 @@ -on: issue_comment - -jobs: - echo-body: - runs-on: ubuntu-latest - steps: - - env: - BODY: ${{ github.event.issue.body }} - run: | - echo '${{ env.BODY }}' \ No newline at end of file diff --git a/javascript/ql/src/Security/CWE-094/examples/comment_issue_good.yml b/javascript/ql/src/Security/CWE-094/examples/comment_issue_good.yml deleted file mode 100644 index 07254a8b204..00000000000 --- a/javascript/ql/src/Security/CWE-094/examples/comment_issue_good.yml +++ /dev/null @@ -1,10 +0,0 @@ -on: issue_comment - -jobs: - echo-body: - runs-on: ubuntu-latest - steps: - - env: - BODY: ${{ github.event.issue.body }} - run: | - echo "$BODY" diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/comment_issue.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/comment_issue.yml deleted file mode 100644 index 17ead9fdd20..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/comment_issue.yml +++ /dev/null @@ -1,28 +0,0 @@ -on: issue_comment - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: | - echo '${{ github.event.comment.body }}' - - echo-chamber2: - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.comment.body }}' - - run: echo '${{ github.event.issue.body }}' - - run: echo '${{ github.event.issue.title }}' - - echo-chamber3: - runs-on: ubuntu-latest - steps: - - uses: actions/github-script@v3 - with: - script: console.log('${{ github.event.comment.body }}') - - uses: actions/github-script@v3 - with: - script: console.log('${{ github.event.issue.body }}') - - uses: actions/github-script@v3 - with: - script: console.log('${{ github.event.issue.title }}') \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/comment_issue_newline.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/comment_issue_newline.yml deleted file mode 100644 index 0a64e47f6cb..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/comment_issue_newline.yml +++ /dev/null @@ -1,10 +0,0 @@ -on: issue_comment - -# same as comment_issue but this file ends with a line break - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: | - echo '${{ github.event.comment.body }}' diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/discussion.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/discussion.yml deleted file mode 100644 index fdb140ec380..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/discussion.yml +++ /dev/null @@ -1,8 +0,0 @@ -on: discussion - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.discussion.title }}' - - run: echo '${{ github.event.discussion.body }}' \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/discussion_comment.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/discussion_comment.yml deleted file mode 100644 index 649d3a6e131..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/discussion_comment.yml +++ /dev/null @@ -1,9 +0,0 @@ -on: discussion_comment - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.discussion.title }}' - - run: echo '${{ github.event.discussion.body }}' - - run: echo '${{ github.event.comment.body }}' \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/gollum.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/gollum.yml deleted file mode 100644 index a952c8c1ab8..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/gollum.yml +++ /dev/null @@ -1,11 +0,0 @@ -on: gollum - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.pages[1].title }}' - - run: echo '${{ github.event.pages[11].title }}' - - run: echo '${{ github.event.pages[0].page_name }}' - - run: echo '${{ github.event.pages[2222].page_name }}' - - run: echo '${{ toJSON(github.event.pages.*.title) }}' # safe \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/issues.yaml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/issues.yaml deleted file mode 100644 index 5e767ce0239..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/issues.yaml +++ /dev/null @@ -1,20 +0,0 @@ -on: issues - -env: - global_env: ${{ github.event.issue.title }} - test: test - -jobs: - echo-chamber: - env: - job_env: ${{ github.event.issue.title }} - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.issue.title }}' - - run: echo '${{ github.event.issue.body }}' - - run: echo '${{ env.global_env }}' - - run: echo '${{ env.test }}' - - run: echo '${{ env.job_env }}' - - run: echo '${{ env.step_env }}' - env: - step_env: ${{ github.event.issue.title }} diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_review.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_review.yml deleted file mode 100644 index d4ce7885669..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_review.yml +++ /dev/null @@ -1,14 +0,0 @@ -on: pull_request_review - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.pull_request.title }}' - - run: echo '${{ github.event.pull_request.body }}' - - run: echo '${{ github.event.pull_request.head.label }}' - - run: echo '${{ github.event.pull_request.head.repo.default_branch }}' - - run: echo '${{ github.event.pull_request.head.repo.description }}' - - run: echo '${{ github.event.pull_request.head.repo.homepage }}' - - run: echo '${{ github.event.pull_request.head.ref }}' - - run: echo '${{ github.event.review.body }}' diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_review_comment.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_review_comment.yml deleted file mode 100644 index 5d288caad85..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_review_comment.yml +++ /dev/null @@ -1,14 +0,0 @@ -on: pull_request_review_comment - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.pull_request.title }}' - - run: echo '${{ github.event.pull_request.body }}' - - run: echo '${{ github.event.pull_request.head.label }}' - - run: echo '${{ github.event.pull_request.head.repo.default_branch }}' - - run: echo '${{ github.event.pull_request.head.repo.description }}' - - run: echo '${{ github.event.pull_request.head.repo.homepage }}' - - run: echo '${{ github.event.pull_request.head.ref }}' - - run: echo '${{ github.event.comment.body }}' diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_target.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_target.yml deleted file mode 100644 index 215b3252885..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/pull_request_target.yml +++ /dev/null @@ -1,16 +0,0 @@ -on: pull_request_target - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.issue.title }}' # not defined - - run: echo '${{ github.event.issue.body }}' # not defined - - run: echo '${{ github.event.pull_request.title }}' - - run: echo '${{ github.event.pull_request.body }}' - - run: echo '${{ github.event.pull_request.head.label }}' - - run: echo '${{ github.event.pull_request.head.repo.default_branch }}' - - run: echo '${{ github.event.pull_request.head.repo.description }}' - - run: echo '${{ github.event.pull_request.head.repo.homepage }}' - - run: echo '${{ github.event.pull_request.head.ref }}' - - run: echo '${{ github.head_ref }}' diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/push.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/push.yml deleted file mode 100644 index 2006a7999da..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/push.yml +++ /dev/null @@ -1,16 +0,0 @@ -on: push - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.commits[11].message }}' - - run: echo '${{ github.event.commits[11].author.email }}' - - run: echo '${{ github.event.commits[11].author.name }}' - - run: echo '${{ github.event.head_commit.message }}' - - run: echo '${{ github.event.head_commit.author.email }}' - - run: echo '${{ github.event.head_commit.author.name }}' - - run: echo '${{ github.event.head_commit.committer.email }}' - - run: echo '${{ github.event.head_commit.committer.name }}' - - run: echo '${{ github.event.commits[11].committer.email }}' - - run: echo '${{ github.event.commits[11].committer.name }}' \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/workflow_run.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/workflow_run.yml deleted file mode 100644 index 60e7645f60f..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/.github/workflows/workflow_run.yml +++ /dev/null @@ -1,16 +0,0 @@ -on: - workflow_run: - workflows: [test] - -jobs: - echo-chamber: - runs-on: ubuntu-latest - steps: - - run: echo '${{ github.event.workflow_run.display_title }}' - - run: echo '${{ github.event.workflow_run.head_commit.message }}' - - run: echo '${{ github.event.workflow_run.head_commit.author.email }}' - - run: echo '${{ github.event.workflow_run.head_commit.author.name }}' - - run: echo '${{ github.event.workflow_run.head_commit.committer.email }}' - - run: echo '${{ github.event.workflow_run.head_commit.committer.name }}' - - run: echo '${{ github.event.workflow_run.head_branch }}' - - run: echo '${{ github.event.workflow_run.head_repository.description }}' diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.expected b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.expected deleted file mode 100644 index 414b9b9ae40..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.expected +++ /dev/null @@ -1,65 +0,0 @@ -| .github/workflows/comment_issue.yml:7:12:8:48 | \| | Potential injection from the ${{ github.event.comment.body }}, which may be controlled by an external user. | -| .github/workflows/comment_issue.yml:13:12:13:50 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.comment.body }}, which may be controlled by an external user. | -| .github/workflows/comment_issue.yml:14:12:14:48 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.issue.body }}, which may be controlled by an external user. | -| .github/workflows/comment_issue.yml:15:12:15:49 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.issue.title }}, which may be controlled by an external user. | -| .github/workflows/comment_issue.yml:22:17:22:63 | console ... dy }}') | Potential injection from the ${{ github.event.comment.body }}, which may be controlled by an external user. | -| .github/workflows/comment_issue.yml:25:17:25:61 | console ... dy }}') | Potential injection from the ${{ github.event.issue.body }}, which may be controlled by an external user. | -| .github/workflows/comment_issue.yml:28:17:28:62 | console ... le }}') | Potential injection from the ${{ github.event.issue.title }}, which may be controlled by an external user. | -| .github/workflows/comment_issue_newline.yml:9:14:10:50 | \| | Potential injection from the ${{ github.event.comment.body }}, which may be controlled by an external user. | -| .github/workflows/discussion.yml:7:12:7:54 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.discussion.title }}, which may be controlled by an external user. | -| .github/workflows/discussion.yml:8:12:8:53 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.discussion.body }}, which may be controlled by an external user. | -| .github/workflows/discussion_comment.yml:7:12:7:54 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.discussion.title }}, which may be controlled by an external user. | -| .github/workflows/discussion_comment.yml:8:12:8:53 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.discussion.body }}, which may be controlled by an external user. | -| .github/workflows/discussion_comment.yml:9:12:9:50 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.comment.body }}, which may be controlled by an external user. | -| .github/workflows/gollum.yml:7:12:7:52 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.pages[1].title }}, which may be controlled by an external user. | -| .github/workflows/gollum.yml:8:12:8:53 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.pages[11].title }}, which may be controlled by an external user. | -| .github/workflows/gollum.yml:9:12:9:56 | echo '$ ... ame }}' | Potential injection from the ${{ github.event.pages[0].page_name }}, which may be controlled by an external user. | -| .github/workflows/gollum.yml:10:12:10:59 | echo '$ ... ame }}' | Potential injection from the ${{ github.event.pages[2222].page_name }}, which may be controlled by an external user. | -| .github/workflows/issues.yaml:13:12:13:49 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.issue.title }}, which may be controlled by an external user. | -| .github/workflows/issues.yaml:14:12:14:48 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.issue.body }}, which may be controlled by an external user. | -| .github/workflows/issues.yaml:15:12:15:39 | echo '$ ... env }}' | Potential injection from the ${{ env.global_env }}, which may be controlled by an external user. | -| .github/workflows/issues.yaml:17:12:17:36 | echo '$ ... env }}' | Potential injection from the ${{ env.job_env }}, which may be controlled by an external user. | -| .github/workflows/issues.yaml:18:12:18:37 | echo '$ ... env }}' | Potential injection from the ${{ env.step_env }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review.yml:7:12:7:56 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.pull_request.title }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review.yml:8:12:8:55 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.pull_request.body }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review.yml:9:12:9:61 | echo '$ ... bel }}' | Potential injection from the ${{ github.event.pull_request.head.label }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review.yml:10:12:10:75 | echo '$ ... nch }}' | Potential injection from the ${{ github.event.pull_request.head.repo.default_branch }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review.yml:11:12:11:72 | echo '$ ... ion }}' | Potential injection from the ${{ github.event.pull_request.head.repo.description }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review.yml:12:12:12:69 | echo '$ ... age }}' | Potential injection from the ${{ github.event.pull_request.head.repo.homepage }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review.yml:13:12:13:59 | echo '$ ... ref }}' | Potential injection from the ${{ github.event.pull_request.head.ref }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review.yml:14:12:14:49 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.review.body }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review_comment.yml:7:12:7:56 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.pull_request.title }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review_comment.yml:8:12:8:55 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.pull_request.body }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review_comment.yml:9:12:9:61 | echo '$ ... bel }}' | Potential injection from the ${{ github.event.pull_request.head.label }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review_comment.yml:10:12:10:75 | echo '$ ... nch }}' | Potential injection from the ${{ github.event.pull_request.head.repo.default_branch }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review_comment.yml:11:12:11:72 | echo '$ ... ion }}' | Potential injection from the ${{ github.event.pull_request.head.repo.description }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review_comment.yml:12:12:12:69 | echo '$ ... age }}' | Potential injection from the ${{ github.event.pull_request.head.repo.homepage }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review_comment.yml:13:12:13:59 | echo '$ ... ref }}' | Potential injection from the ${{ github.event.pull_request.head.ref }}, which may be controlled by an external user. | -| .github/workflows/pull_request_review_comment.yml:14:12:14:50 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.comment.body }}, which may be controlled by an external user. | -| .github/workflows/pull_request_target.yml:9:12:9:56 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.pull_request.title }}, which may be controlled by an external user. | -| .github/workflows/pull_request_target.yml:10:12:10:55 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.pull_request.body }}, which may be controlled by an external user. | -| .github/workflows/pull_request_target.yml:11:12:11:61 | echo '$ ... bel }}' | Potential injection from the ${{ github.event.pull_request.head.label }}, which may be controlled by an external user. | -| .github/workflows/pull_request_target.yml:12:12:12:75 | echo '$ ... nch }}' | Potential injection from the ${{ github.event.pull_request.head.repo.default_branch }}, which may be controlled by an external user. | -| .github/workflows/pull_request_target.yml:13:12:13:72 | echo '$ ... ion }}' | Potential injection from the ${{ github.event.pull_request.head.repo.description }}, which may be controlled by an external user. | -| .github/workflows/pull_request_target.yml:14:12:14:69 | echo '$ ... age }}' | Potential injection from the ${{ github.event.pull_request.head.repo.homepage }}, which may be controlled by an external user. | -| .github/workflows/pull_request_target.yml:15:12:15:59 | echo '$ ... ref }}' | Potential injection from the ${{ github.event.pull_request.head.ref }}, which may be controlled by an external user. | -| .github/workflows/pull_request_target.yml:16:12:16:40 | echo '$ ... ref }}' | Potential injection from the ${{ github.head_ref }}, which may be controlled by an external user. | -| .github/workflows/push.yml:7:12:7:57 | echo '$ ... age }}' | Potential injection from the ${{ github.event.commits[11].message }}, which may be controlled by an external user. | -| .github/workflows/push.yml:8:12:8:62 | echo '$ ... ail }}' | Potential injection from the ${{ github.event.commits[11].author.email }}, which may be controlled by an external user. | -| .github/workflows/push.yml:9:12:9:61 | echo '$ ... ame }}' | Potential injection from the ${{ github.event.commits[11].author.name }}, which may be controlled by an external user. | -| .github/workflows/push.yml:10:12:10:57 | echo '$ ... age }}' | Potential injection from the ${{ github.event.head_commit.message }}, which may be controlled by an external user. | -| .github/workflows/push.yml:11:12:11:62 | echo '$ ... ail }}' | Potential injection from the ${{ github.event.head_commit.author.email }}, which may be controlled by an external user. | -| .github/workflows/push.yml:12:12:12:61 | echo '$ ... ame }}' | Potential injection from the ${{ github.event.head_commit.author.name }}, which may be controlled by an external user. | -| .github/workflows/push.yml:13:12:13:65 | echo '$ ... ail }}' | Potential injection from the ${{ github.event.head_commit.committer.email }}, which may be controlled by an external user. | -| .github/workflows/push.yml:14:12:14:64 | echo '$ ... ame }}' | Potential injection from the ${{ github.event.head_commit.committer.name }}, which may be controlled by an external user. | -| .github/workflows/push.yml:15:12:15:65 | echo '$ ... ail }}' | Potential injection from the ${{ github.event.commits[11].committer.email }}, which may be controlled by an external user. | -| .github/workflows/push.yml:16:12:16:64 | echo '$ ... ame }}' | Potential injection from the ${{ github.event.commits[11].committer.name }}, which may be controlled by an external user. | -| .github/workflows/workflow_run.yml:9:12:9:64 | echo '$ ... tle }}' | Potential injection from the ${{ github.event.workflow_run.display_title }}, which may be controlled by an external user. | -| .github/workflows/workflow_run.yml:10:12:10:70 | echo '$ ... age }}' | Potential injection from the ${{ github.event.workflow_run.head_commit.message }}, which may be controlled by an external user. | -| .github/workflows/workflow_run.yml:11:12:11:75 | echo '$ ... ail }}' | Potential injection from the ${{ github.event.workflow_run.head_commit.author.email }}, which may be controlled by an external user. | -| .github/workflows/workflow_run.yml:12:12:12:74 | echo '$ ... ame }}' | Potential injection from the ${{ github.event.workflow_run.head_commit.author.name }}, which may be controlled by an external user. | -| .github/workflows/workflow_run.yml:13:12:13:78 | echo '$ ... ail }}' | Potential injection from the ${{ github.event.workflow_run.head_commit.committer.email }}, which may be controlled by an external user. | -| .github/workflows/workflow_run.yml:14:12:14:77 | echo '$ ... ame }}' | Potential injection from the ${{ github.event.workflow_run.head_commit.committer.name }}, which may be controlled by an external user. | -| .github/workflows/workflow_run.yml:15:12:15:62 | echo '$ ... nch }}' | Potential injection from the ${{ github.event.workflow_run.head_branch }}, which may be controlled by an external user. | -| .github/workflows/workflow_run.yml:16:12:16:78 | echo '$ ... ion }}' | Potential injection from the ${{ github.event.workflow_run.head_repository.description }}, which may be controlled by an external user. | -| action1/action.yml:14:12:14:50 | echo '$ ... ody }}' | Potential injection from the ${{ github.event.comment.body }}, which may be controlled by an external user. | diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.qlref b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.qlref deleted file mode 100644 index dd00277b79b..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/ExpressionInjection.qlref +++ /dev/null @@ -1 +0,0 @@ -query: Security/CWE-094/ExpressionInjection.ql diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/action1/action.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/action1/action.yml deleted file mode 100644 index e9a178cf3db..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/action1/action.yml +++ /dev/null @@ -1,14 +0,0 @@ -name: 'test' -description: 'test' -branding: - icon: 'test' - color: 'test' -inputs: - test: - description: test - required: false - default: 'test' -runs: - using: "composite" - steps: - - run: echo '${{ github.event.comment.body }}' \ No newline at end of file diff --git a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/action2/action.yml b/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/action2/action.yml deleted file mode 100644 index 40fe0b31e6a..00000000000 --- a/javascript/ql/test/query-tests/Security/CWE-094/ExpressionInjection/action2/action.yml +++ /dev/null @@ -1,17 +0,0 @@ -name: 'Hello World' -description: 'Greet someone and record the time' -inputs: - who-to-greet: # id of input - description: 'Who to greet' - required: true - default: 'World' -outputs: - time: # id of output - description: 'The time we greeted you' -runs: - using: 'docker' - steps: # this is actually invalid, used to test we correctly identify composite actions - - run: echo '${{ github.event.comment.body }}' - image: 'Dockerfile' - args: - - ${{ inputs.who-to-greet }} \ No newline at end of file From b1da23968c0a27ad8e62ffb1e1629501bb0095a9 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 23 Jun 2025 14:46:05 +0200 Subject: [PATCH 5/8] JS: Change note --- .../2025-06-23-remove-legacy-actions-queries.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 javascript/ql/src/change-notes/2025-06-23-remove-legacy-actions-queries.md diff --git a/javascript/ql/src/change-notes/2025-06-23-remove-legacy-actions-queries.md b/javascript/ql/src/change-notes/2025-06-23-remove-legacy-actions-queries.md new file mode 100644 index 00000000000..412a941b878 --- /dev/null +++ b/javascript/ql/src/change-notes/2025-06-23-remove-legacy-actions-queries.md @@ -0,0 +1,7 @@ +--- +category: minorAnalysis +--- +* Removed three queries from the JS qlpack, which have been superseded by newer queries that are part of the Actions qlpack: + * `js/actions/pull-request-target` has been superseded by `actions/untrusted-checkout/{medium,high,critical}` + * `js/actions/actions-artifact-leak` has been supersded by `actions/secrets-in-artifacts` + * `js/actions/command-injection` has been superseded by `actions/command-injection/{medium,critical}` From 7da2d71a7038313b86461a0dcc8f3d876042cc7a Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 23 Jun 2025 14:57:23 +0200 Subject: [PATCH 6/8] JS: Update query suite expectations --- .../query-suite/javascript-code-scanning.qls.expected | 2 -- .../query-suite/javascript-security-and-quality.qls.expected | 2 -- .../query-suite/javascript-security-extended.qls.expected | 2 -- .../integration-tests/query-suite/not_included_in_qls.expected | 1 - 4 files changed, 7 deletions(-) diff --git a/javascript/ql/integration-tests/query-suite/javascript-code-scanning.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-code-scanning.qls.expected index 1cf124ce3cf..652ac0ebc1b 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-code-scanning.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-code-scanning.qls.expected @@ -31,7 +31,6 @@ ql/javascript/ql/src/Security/CWE-079/Xss.ql ql/javascript/ql/src/Security/CWE-079/XssThroughDom.ql ql/javascript/ql/src/Security/CWE-089/SqlInjection.ql ql/javascript/ql/src/Security/CWE-094/CodeInjection.ql -ql/javascript/ql/src/Security/CWE-094/ExpressionInjection.ql ql/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql ql/javascript/ql/src/Security/CWE-094/UnsafeDynamicMethodAccess.ql ql/javascript/ql/src/Security/CWE-1004/ClientExposedCookie.ql @@ -48,7 +47,6 @@ ql/javascript/ql/src/Security/CWE-201/PostMessageStar.ql ql/javascript/ql/src/Security/CWE-209/StackTraceExposure.ql ql/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql ql/javascript/ql/src/Security/CWE-300/InsecureDependencyResolution.ql -ql/javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.ql ql/javascript/ql/src/Security/CWE-312/BuildArtifactLeak.ql ql/javascript/ql/src/Security/CWE-312/CleartextLogging.ql ql/javascript/ql/src/Security/CWE-312/CleartextStorage.ql diff --git a/javascript/ql/integration-tests/query-suite/javascript-security-and-quality.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-security-and-quality.qls.expected index eb4acd38e39..dd587768308 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-security-and-quality.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-security-and-quality.qls.expected @@ -119,7 +119,6 @@ ql/javascript/ql/src/Security/CWE-079/Xss.ql ql/javascript/ql/src/Security/CWE-079/XssThroughDom.ql ql/javascript/ql/src/Security/CWE-089/SqlInjection.ql ql/javascript/ql/src/Security/CWE-094/CodeInjection.ql -ql/javascript/ql/src/Security/CWE-094/ExpressionInjection.ql ql/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql ql/javascript/ql/src/Security/CWE-094/UnsafeCodeConstruction.ql ql/javascript/ql/src/Security/CWE-094/UnsafeDynamicMethodAccess.ql @@ -140,7 +139,6 @@ ql/javascript/ql/src/Security/CWE-201/PostMessageStar.ql ql/javascript/ql/src/Security/CWE-209/StackTraceExposure.ql ql/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql ql/javascript/ql/src/Security/CWE-300/InsecureDependencyResolution.ql -ql/javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.ql ql/javascript/ql/src/Security/CWE-312/BuildArtifactLeak.ql ql/javascript/ql/src/Security/CWE-312/CleartextLogging.ql ql/javascript/ql/src/Security/CWE-312/CleartextStorage.ql diff --git a/javascript/ql/integration-tests/query-suite/javascript-security-extended.qls.expected b/javascript/ql/integration-tests/query-suite/javascript-security-extended.qls.expected index a5b5cfefdbc..9b7cfd22ed6 100644 --- a/javascript/ql/integration-tests/query-suite/javascript-security-extended.qls.expected +++ b/javascript/ql/integration-tests/query-suite/javascript-security-extended.qls.expected @@ -34,7 +34,6 @@ ql/javascript/ql/src/Security/CWE-079/Xss.ql ql/javascript/ql/src/Security/CWE-079/XssThroughDom.ql ql/javascript/ql/src/Security/CWE-089/SqlInjection.ql ql/javascript/ql/src/Security/CWE-094/CodeInjection.ql -ql/javascript/ql/src/Security/CWE-094/ExpressionInjection.ql ql/javascript/ql/src/Security/CWE-094/ImproperCodeSanitization.ql ql/javascript/ql/src/Security/CWE-094/UnsafeCodeConstruction.ql ql/javascript/ql/src/Security/CWE-094/UnsafeDynamicMethodAccess.ql @@ -55,7 +54,6 @@ ql/javascript/ql/src/Security/CWE-201/PostMessageStar.ql ql/javascript/ql/src/Security/CWE-209/StackTraceExposure.ql ql/javascript/ql/src/Security/CWE-295/DisablingCertificateValidation.ql ql/javascript/ql/src/Security/CWE-300/InsecureDependencyResolution.ql -ql/javascript/ql/src/Security/CWE-312/ActionsArtifactLeak.ql ql/javascript/ql/src/Security/CWE-312/BuildArtifactLeak.ql ql/javascript/ql/src/Security/CWE-312/CleartextLogging.ql ql/javascript/ql/src/Security/CWE-312/CleartextStorage.ql diff --git a/javascript/ql/integration-tests/query-suite/not_included_in_qls.expected b/javascript/ql/integration-tests/query-suite/not_included_in_qls.expected index c80c3fc76da..9d67dfc2cc7 100644 --- a/javascript/ql/integration-tests/query-suite/not_included_in_qls.expected +++ b/javascript/ql/integration-tests/query-suite/not_included_in_qls.expected @@ -67,7 +67,6 @@ ql/javascript/ql/src/Summary/TaintSinks.ql ql/javascript/ql/src/Summary/TaintSources.ql ql/javascript/ql/src/definitions.ql ql/javascript/ql/src/experimental/Security/CWE-094-dataURL/CodeInjection.ql -ql/javascript/ql/src/experimental/Security/CWE-094/UntrustedCheckout.ql ql/javascript/ql/src/experimental/Security/CWE-099/EnvValueAndKeyInjection.ql ql/javascript/ql/src/experimental/Security/CWE-099/EnvValueInjection.ql ql/javascript/ql/src/experimental/Security/CWE-340/TokenBuiltFromUUID.ql From ea0a80a06aabdad698c5631911811431bd3449c8 Mon Sep 17 00:00:00 2001 From: Asger F Date: Mon, 23 Jun 2025 16:38:04 +0200 Subject: [PATCH 7/8] JS: Un-deprecate Actions.qll for now as we have some internal queries that use it. --- javascript/ql/lib/semmle/javascript/Actions.qll | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/javascript/ql/lib/semmle/javascript/Actions.qll b/javascript/ql/lib/semmle/javascript/Actions.qll index d3f19681d88..a3dd7542ec1 100644 --- a/javascript/ql/lib/semmle/javascript/Actions.qll +++ b/javascript/ql/lib/semmle/javascript/Actions.qll @@ -1,10 +1,9 @@ /** - * DEPRECATED. Models for GitHub Actions workflow files are part of the actions qlpack now. + * PENDING DEPRECATION. Models for GitHub Actions workflow files are part of the actions qlpack now. * * Libraries for modeling GitHub Actions workflow files written in YAML. * See https://docs.github.com/en/actions/reference/workflow-syntax-for-github-actions. */ -deprecated module; import javascript From 54bfde9b7af86accf055d64d4bc485d0aedbb531 Mon Sep 17 00:00:00 2001 From: Asger F Date: Tue, 24 Jun 2025 11:22:37 +0200 Subject: [PATCH 8/8] Update javascript/ql/src/change-notes/2025-06-23-remove-legacy-actions-queries.md Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- .../change-notes/2025-06-23-remove-legacy-actions-queries.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/javascript/ql/src/change-notes/2025-06-23-remove-legacy-actions-queries.md b/javascript/ql/src/change-notes/2025-06-23-remove-legacy-actions-queries.md index 412a941b878..628ad8b083b 100644 --- a/javascript/ql/src/change-notes/2025-06-23-remove-legacy-actions-queries.md +++ b/javascript/ql/src/change-notes/2025-06-23-remove-legacy-actions-queries.md @@ -3,5 +3,5 @@ category: minorAnalysis --- * Removed three queries from the JS qlpack, which have been superseded by newer queries that are part of the Actions qlpack: * `js/actions/pull-request-target` has been superseded by `actions/untrusted-checkout/{medium,high,critical}` - * `js/actions/actions-artifact-leak` has been supersded by `actions/secrets-in-artifacts` + * `js/actions/actions-artifact-leak` has been superseded by `actions/secrets-in-artifacts` * `js/actions/command-injection` has been superseded by `actions/command-injection/{medium,critical}`