From 778d8978b05267a6b1eed1df7d5e03cb8ea9f8a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= Date: Thu, 14 Mar 2024 13:55:10 +0100 Subject: [PATCH] DF support for untrusted checkout query --- .../dataflow/internal/DataFlowPublic.qll | 1 + ql/src/Security/CWE-829/UntrustedCheckout.ql | 18 ++++++++----- .../issue_comment_3rd_party_action.yml | 1 - .../.github/workflows/untrusted_checkout.yml | 26 ++++++------------- .../CWE-829/UnpinnedActionsTag.expected | 8 +++--- .../CWE-829/UntrustedCheckout.expected | 11 ++++---- 6 files changed, 30 insertions(+), 35 deletions(-) diff --git a/ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll b/ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll index 8e8ed5d9280..681d6f1cfc3 100644 --- a/ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll +++ b/ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll @@ -176,6 +176,7 @@ class FieldContent extends Content, TFieldContent { } predicate hasLocalFlow(Node n1, Node n2) { + n1 = n2 or simpleLocalFlowStep(n1, n2) or exists(ContentSet c | ctxFieldReadStep(n1, n2, c)) } diff --git a/ql/src/Security/CWE-829/UntrustedCheckout.ql b/ql/src/Security/CWE-829/UntrustedCheckout.ql index f12f1102087..1be8a6ea0f5 100644 --- a/ql/src/Security/CWE-829/UntrustedCheckout.ql +++ b/ql/src/Security/CWE-829/UntrustedCheckout.ql @@ -79,8 +79,19 @@ class ActionsCheckout extends PRHeadCheckoutStep instanceof UsesStep { ActionsCheckout() { this.getCallee() = "actions/checkout" and ( - containsHeadRef(this.getArgumentExpr("ref").getExpression()) + // ref argument contains the head ref + exists(Expression e | + containsHeadRef(e.getExpression()) and + DataFlow::hasLocalFlowExpr(e, this.getArgumentExpr("ref")) + ) or + // 3rd party actions returning the PR head sha/ref + exists(UsesStep head | + head.getCallee() = ["eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch"] and + DataFlow::hasLocalFlowExpr(head, this.getArgumentExpr("ref")) + ) + or + // heuristic base on the step id and field name exists(StepsExpression e | this.getArgumentExpr("ref") = e and ( @@ -88,11 +99,6 @@ class ActionsCheckout extends PRHeadCheckoutStep instanceof UsesStep { e.getFieldName().matches(["%sha%", "%head%", "branch"]) ) ) - or - exists(UsesStep head | - head.getCallee() = ["eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch"] and - DataFlow::hasLocalFlowExpr(head, this.getArgumentExpr("ref")) - ) ) } } diff --git a/ql/test/query-tests/Security/CWE-829/.github/workflows/issue_comment_3rd_party_action.yml b/ql/test/query-tests/Security/CWE-829/.github/workflows/issue_comment_3rd_party_action.yml index 4de47d6f17a..221854ec204 100644 --- a/ql/test/query-tests/Security/CWE-829/.github/workflows/issue_comment_3rd_party_action.yml +++ b/ql/test/query-tests/Security/CWE-829/.github/workflows/issue_comment_3rd_party_action.yml @@ -13,7 +13,6 @@ jobs: if: ${{ github.event_name == 'issue_comment' }} uses: xt0rted/pull-request-comment-branch@v2 id: comment-branch - - name: (PR comment) Checkout PR branch if: ${{ github.event_name == 'issue_comment' }} uses: actions/checkout@v3 diff --git a/ql/test/query-tests/Security/CWE-829/.github/workflows/untrusted_checkout.yml b/ql/test/query-tests/Security/CWE-829/.github/workflows/untrusted_checkout.yml index a37ceb8f9f6..6bcdcbb4291 100644 --- a/ql/test/query-tests/Security/CWE-829/.github/workflows/untrusted_checkout.yml +++ b/ql/test/query-tests/Security/CWE-829/.github/workflows/untrusted_checkout.yml @@ -3,23 +3,13 @@ on: jobs: build: - name: Build and test runs-on: ubuntu-latest + env: + HEAD: ${{ github.event.pull_request.head.sha }} 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! + - uses: actions/checkout@v2 + with: + ref: ${{ github.event.pull_request.head.sha }} + - uses: actions/checkout@v2 + with: + ref: ${{ env.HEAD }} diff --git a/ql/test/query-tests/Security/CWE-829/UnpinnedActionsTag.expected b/ql/test/query-tests/Security/CWE-829/UnpinnedActionsTag.expected index 48b7b762605..c3a3ec2f988 100644 --- a/ql/test/query-tests/Security/CWE-829/UnpinnedActionsTag.expected +++ b/ql/test/query-tests/Security/CWE-829/UnpinnedActionsTag.expected @@ -3,14 +3,12 @@ | .github/workflows/auto_ci.yml:93:9:96:6 | Uses Step | Unpinned 3rd party Action 'Python CI' step $@ uses 'codecov/codecov-action' with ref 'v3', not a pinned commit hash | .github/workflows/auto_ci.yml:93:9:96:6 | Uses Step | Uses Step | | .github/workflows/auto_ci.yml:108:9:119:6 | Uses Step: create_pr | Unpinned 3rd party Action 'Python CI' step $@ uses 'peter-evans/create-pull-request' with ref 'v5', not a pinned commit hash | .github/workflows/auto_ci.yml:108:9:119:6 | Uses Step: create_pr | Uses Step: create_pr | | .github/workflows/auto_ci.yml:125:9:133:6 | Uses Step | Unpinned 3rd party Action 'Python CI' step $@ uses 'thollander/actions-comment-pull-request' with ref 'v2', not a pinned commit hash | .github/workflows/auto_ci.yml:125:9:133:6 | Uses Step | Uses Step | -| .github/workflows/issue_comment_3rd_party_action.yml:12:9:17:6 | Uses Step: comment-branch | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'xt0rted/pull-request-comment-branch' with ref 'v2', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:12:9:17:6 | Uses Step: comment-branch | Uses Step: comment-branch | -| .github/workflows/issue_comment_3rd_party_action.yml:26:9:31:6 | Uses Step: comment-branch | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'xt0rted/pull-request-comment-branch' with ref 'v2', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:26:9:31:6 | Uses Step: comment-branch | Uses Step: comment-branch | -| .github/workflows/issue_comment_3rd_party_action.yml:40:9:46:6 | Uses Step: refs | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'eficode/resolve-pr-refs' with ref 'main', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:40:9:46:6 | Uses Step: refs | Uses Step: refs | +| .github/workflows/issue_comment_3rd_party_action.yml:12:9:16:6 | Uses Step: comment-branch | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'xt0rted/pull-request-comment-branch' with ref 'v2', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:12:9:16:6 | Uses Step: comment-branch | Uses Step: comment-branch | +| .github/workflows/issue_comment_3rd_party_action.yml:25:9:30:6 | Uses Step: comment-branch | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'xt0rted/pull-request-comment-branch' with ref 'v2', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:25:9:30:6 | Uses Step: comment-branch | Uses Step: comment-branch | +| .github/workflows/issue_comment_3rd_party_action.yml:39:9:45:6 | Uses Step: refs | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'eficode/resolve-pr-refs' with ref 'main', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:39:9:45:6 | Uses Step: refs | Uses Step: refs | | .github/workflows/issue_comment_octokit.yml:12:9:19:6 | Uses Step: fetch_issue | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'octokit/request-action' with ref 'v2.x', not a pinned commit hash | .github/workflows/issue_comment_octokit.yml:12:9:19:6 | Uses Step: fetch_issue | Uses Step: fetch_issue | | .github/workflows/issue_comment_octokit.yml:19:9:26:6 | Uses Step: fetch_pr | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'octokit/request-action' with ref 'v2.x', not a pinned commit hash | .github/workflows/issue_comment_octokit.yml:19:9:26:6 | Uses Step: fetch_pr | Uses Step: fetch_pr | | .github/workflows/issue_comment_octokit.yml:103:9:109:6 | Uses Step: request | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'octokit/request-action' with ref 'v2.0.2', not a pinned commit hash | .github/workflows/issue_comment_octokit.yml:103:9:109:6 | Uses Step: request | Uses Step: request | | .github/workflows/label_trusted_checkout.yml:20:7:24:4 | Uses Step | Unpinned 3rd party Action 'label_trusted_checkout.yml' step $@ uses 'completely/fakeaction' with ref 'v2', not a pinned commit hash | .github/workflows/label_trusted_checkout.yml:20:7:24:4 | Uses Step | Uses Step | | .github/workflows/label_trusted_checkout.yml:24:7:27:21 | Uses Step | Unpinned 3rd party Action 'label_trusted_checkout.yml' step $@ uses 'fakerepo/comment-on-pr' with ref 'v1', not a pinned commit hash | .github/workflows/label_trusted_checkout.yml:24:7:27:21 | Uses Step | Uses Step | | .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | Unpinned 3rd party Action 'unpinned_tags.yml' step $@ uses 'foo/bar' with ref 'v1', not a pinned commit hash | .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | Uses Step | -| .github/workflows/untrusted_checkout.yml:18:7:22:4 | Uses Step | Unpinned 3rd party Action 'untrusted_checkout.yml' step $@ uses 'completely/fakeaction' with ref 'v2', not a pinned commit hash | .github/workflows/untrusted_checkout.yml:18:7:22:4 | Uses Step | Uses Step | -| .github/workflows/untrusted_checkout.yml:22:7:25:21 | Uses Step | Unpinned 3rd party Action 'untrusted_checkout.yml' step $@ uses 'fakerepo/comment-on-pr' with ref 'v1', not a pinned commit hash | .github/workflows/untrusted_checkout.yml:22:7:25:21 | Uses Step | Uses Step | diff --git a/ql/test/query-tests/Security/CWE-829/UntrustedCheckout.expected b/ql/test/query-tests/Security/CWE-829/UntrustedCheckout.expected index a6f02e7752a..cf9d6c01d49 100644 --- a/ql/test/query-tests/Security/CWE-829/UntrustedCheckout.expected +++ b/ql/test/query-tests/Security/CWE-829/UntrustedCheckout.expected @@ -1,10 +1,10 @@ | .github/workflows/auto_ci.yml:20:9:27:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | | .github/workflows/auto_ci.yml:67:9:74:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | | .github/workflows/gitcheckout.yml:10:11:18:8 | Run Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | -| .github/workflows/issue_comment_3rd_party_action.yml:17:9:23:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | -| .github/workflows/issue_comment_3rd_party_action.yml:31:9:37:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | -| .github/workflows/issue_comment_3rd_party_action.yml:46:9:50:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | -| .github/workflows/issue_comment_3rd_party_action.yml:50:9:53:25 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | +| .github/workflows/issue_comment_3rd_party_action.yml:16:9:22:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | +| .github/workflows/issue_comment_3rd_party_action.yml:30:9:36:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | +| .github/workflows/issue_comment_3rd_party_action.yml:45:9:49:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | +| .github/workflows/issue_comment_3rd_party_action.yml:49:9:52:25 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | | .github/workflows/issue_comment_direct.yml:12:9:16:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | | .github/workflows/issue_comment_direct.yml:20:9:24:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | | .github/workflows/issue_comment_direct.yml:28:9:32:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | @@ -18,4 +18,5 @@ | .github/workflows/issue_comment_octokit.yml:79:9:83:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | | .github/workflows/issue_comment_octokit.yml:95:9:100:2 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | | .github/workflows/issue_comment_octokit.yml:109:9:114:66 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | -| .github/workflows/untrusted_checkout.yml:9:7:13:4 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | +| .github/workflows/untrusted_checkout.yml:10:9:13:6 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. | +| .github/workflows/untrusted_checkout.yml:13:9:15:31 | Uses Step | Potential unsafe checkout of untrusted pull request on privileged workflow. |