diff --git a/ql/src/Security/CWE-829/UntrustedCheckout.ql b/ql/src/Security/CWE-829/UntrustedCheckout.ql index 1be8a6ea0f5..b33c7325526 100644 --- a/ql/src/Security/CWE-829/UntrustedCheckout.ql +++ b/ql/src/Security/CWE-829/UntrustedCheckout.ql @@ -1,6 +1,6 @@ /** * @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. + * @description Priveleged workflows 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 @@ -121,12 +121,26 @@ class GitCheckout extends PRHeadCheckoutStep instanceof Run { } } +predicate isSingleTriggerWorkflow(Workflow w, string trigger) { + w.getATriggerEvent() = trigger and + count(string t | w.getATriggerEvent() = t | t) = 1 +} + from Workflow w, PRHeadCheckoutStep checkout where - w.hasTriggerEvent([ - "pull_request_target", "issue_comment", "pull_request_review_comment", "pull_request_review", - "workflow_run", "check_run", "check_suite", "workflow_call" - ]) and + ( + // The Workflow is triggered by an event other than `pull_request` + not isSingleTriggerWorkflow(w, "pull_request") + or + // The Workflow is only triggered by `workflow_call` and there is + // a caller workflow triggered by an event other than `pull_request` + isSingleTriggerWorkflow(w, "workflow_call") and + exists(ExternalJob call, Workflow caller | + call.getCallee() = w.getLocation().getFile().getRelativePath() and + caller = call.getWorkflow() and + not isSingleTriggerWorkflow(caller, "pull_request") + ) + ) and w.getAJob().(LocalJob).getAStep() = checkout and not exists(ControlCheck check | checkout.getIf() = check or checkout.getEnclosingJob().getIf() = check diff --git a/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_from_prt.yml b/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_from_prt.yml new file mode 100644 index 00000000000..0ee850f183d --- /dev/null +++ b/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_from_prt.yml @@ -0,0 +1,100 @@ +name: changelog + +on: + workflow_call: + inputs: + create: + description: Add a log to the changelog + type: boolean + required: false + default: false + update: + description: Update the existing changelog + type: boolean + required: false + default: false + +jobs: + changelog: + runs-on: ubuntu-latest + env: + file: CHANGELOG.md + steps: + - uses: actions/checkout@v3 + with: + fetch-depth: 0 + - name: Check ${{ env.file }} + run: | + if [[ $(git diff --name-only origin/master HEAD -- ${{ env.file }} | grep '^${{ env.file }}$' -c) -eq 0 ]]; then + echo "Expected '${{ env.file }}' to be modified" + exit 1 + fi + update: + runs-on: ubuntu-latest + needs: changelog + if: (inputs.create && failure()) || (inputs.update && success()) + continue-on-error: true + env: + file: CHANGELOG.md + next_version: next + link: '[#${{ github.event.number }}](https://github.com/fabricjs/fabric.js/pull/${{ github.event.number }})' + steps: + - uses: actions/checkout@v3 + with: + ref: ${{ github.event.pull_request.head.ref }} + - name: Update ${{ env.file }} from PR title + id: update + uses: actions/github-script@v6 + env: + log: '- ${{ github.event.pull_request.title }} ${{ env.link }}\n' + prev_log: '- ${{ github.event.changes.title.from }} ${{ env.link }}\n' + with: + result-encoding: string + script: | + const fs = require('fs'); + const file = './${{ env.file }}'; + let content = fs.readFileSync(file).toString(); + const title = '[${{ env.next_version }}]'; + const log = '${{ env.log }}'; + let exists = ${{ needs.changelog.result == 'success' }}; + + if (!content.includes(title)) { + const insertAt = content.indexOf('\n') + 1; + content = + content.slice(0, insertAt) + + `\n## ${title}\n\n\n` + + content.slice(insertAt); + } + + const insertAt = content.indexOf('\n', content.indexOf(title) + title.length + 1) + 1; + if (exists && ${{ github.event.action == 'edited' }}) { + const prevLog = '${{ env.prev_log }}'; + const index = content.indexOf(prevLog, insertAt); + if (index > -1) { + content = content.slice(0, index) + content.slice(index + prevLog.length); + exists = false; + } + } + + if (!exists) { + content = content.slice(0, insertAt) + log + content.slice(insertAt); + fs.writeFileSync(file, content); + return true; + } + + return false; + - name: Setup node + if: fromJson(steps.update.outputs.result) + uses: actions/setup-node@v3 + with: + node-version: 18.x + - name: Commit & Push + if: fromJson(steps.update.outputs.result) + run: | + npm ci + npx prettier --write ${{ env.file }} + git config user.name github-actions[bot] + git config user.email github-actions[bot]@users.noreply.github.com + git add ${{ env.file }} + git commit -m "update ${{ env.file }}" + git push diff --git a/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_required_prt.yml b/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_required_prt.yml new file mode 100644 index 00000000000..8a3b1b02a63 --- /dev/null +++ b/ql/test/query-tests/Security/CWE-094/.github/workflows/changelog_required_prt.yml @@ -0,0 +1,9 @@ +name: '📋' + +on: + pull_request_target: + branches: [master] + +jobs: + changelog: + uses: ./.github/workflows/changelog_from_prt.yml diff --git a/ql/test/query-tests/Security/CWE-094/CodeInjection.expected b/ql/test/query-tests/Security/CWE-094/CodeInjection.expected index 14b0c535ac6..2ad85054803 100644 --- a/ql/test/query-tests/Security/CWE-094/CodeInjection.expected +++ b/ql/test/query-tests/Security/CWE-094/CodeInjection.expected @@ -4,6 +4,7 @@ edges | .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | | .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | +| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced | | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:57:29:57:73 | steps.remove_quotations.outputs.replaced | | .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | @@ -66,6 +67,8 @@ nodes | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | semmle.label | steps.changed-files.outputs.all_changed_files | | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title | | .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log | +| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title | +| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | semmle.label | env.log | | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | semmle.label | github.event.comment.body | | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | semmle.label | github.event.comment.body | | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | semmle.label | github.event.issue.body | @@ -205,6 +208,7 @@ subpaths | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | ${{steps.remove_quotations.outputs.replaced}} | | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | ${{ steps.changed-files.outputs.all_changed_files }} | | .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} | +| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | ${{ env.log }} | | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | ${{ github.event.comment.body }} | | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | ${{ github.event.comment.body }} | | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | ${{ github.event.issue.body }} | diff --git a/ql/test/query-tests/Security/CWE-094/PrivilegedCodeInjection.expected b/ql/test/query-tests/Security/CWE-094/PrivilegedCodeInjection.expected index bdb5ae3ea55..e818ced0c1d 100644 --- a/ql/test/query-tests/Security/CWE-094/PrivilegedCodeInjection.expected +++ b/ql/test/query-tests/Security/CWE-094/PrivilegedCodeInjection.expected @@ -4,6 +4,7 @@ edges | .github/workflows/argus_case_study.yml:22:20:22:39 | env.ISSUE_TITLE | .github/workflows/argus_case_study.yml:15:9:24:6 | Uses Step: remove_quotations [replaced] | | .github/workflows/changed-files.yml:16:9:20:6 | Uses Step: changed-files | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | +| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced | | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:57:29:57:73 | steps.remove_quotations.outputs.replaced | | .github/workflows/cross3.yml:32:18:32:53 | github.event.commits[0].message | .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | @@ -66,6 +67,8 @@ nodes | .github/workflows/changed-files.yml:22:24:22:75 | steps.changed-files.outputs.all_changed_files | semmle.label | steps.changed-files.outputs.all_changed_files | | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title | | .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log | +| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title | +| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | semmle.label | env.log | | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | semmle.label | github.event.comment.body | | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | semmle.label | github.event.comment.body | | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | semmle.label | github.event.issue.body | @@ -204,6 +207,7 @@ subpaths #select | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | .github/workflows/argus_case_study.yml:17:25:17:53 | github.event.issue.title | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/argus_case_study.yml:27:33:27:77 | steps.remove_quotations.outputs.replaced | ${{steps.remove_quotations.outputs.replaced}} | | .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} | +| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | ${{ env.log }} | | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:15:9:46 | github.event.comment.body | ${{ github.event.comment.body }} | | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:15:19:15:50 | github.event.comment.body | ${{ github.event.comment.body }} | | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | Potential privileged code injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:16:19:16:48 | github.event.issue.body | ${{ github.event.issue.body }} |