DF support for untrusted checkout query

Alvaro Muñoz · Alvaro Muñoz · commit 778d8978b052 · 2024-03-14T13:55:10.000+01:00
diff --git 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
@@ -79,20 +79,26 @@ 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
         (
           e.getStepId().matches(["%sha%", "%head%", "branch"]) or
           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
@@ -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
@@ -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
@@ -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
@@ -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. |

Original file line number	Diff line number	Diff line change
`@@ -176,6 +176,7 @@ class FieldContent extends Content, TFieldContent {`
`176`	`176`	`}`
`177`	`177`
`178`	`178`	`predicate hasLocalFlow(Node n1, Node n2) {`
	`179`	`+ n1 = n2 or`
`179`	`180`	`simpleLocalFlowStep(n1, n2) or`
`180`	`181`	`exists(ContentSet c \| ctxFieldReadStep(n1, n2, c))`
`181`	`182`	`}`
Original file line number	Diff line number	Diff line change
`@@ -79,20 +79,26 @@ class ActionsCheckout extends PRHeadCheckoutStep instanceof UsesStep {`
`79`	`79`	`ActionsCheckout() {`
`80`	`80`	`this.getCallee() = "actions/checkout" and`
`81`	`81`	`(`
`82`		`- containsHeadRef(this.getArgumentExpr("ref").getExpression())`
	`82`	`+ // ref argument contains the head ref`
	`83`	`+ exists(Expression e \|`
	`84`	`+ containsHeadRef(e.getExpression()) and`
	`85`	`+ DataFlow::hasLocalFlowExpr(e, this.getArgumentExpr("ref"))`
	`86`	`+ )`
	`87`	`+ or`
	`88`	`+ // 3rd party actions returning the PR head sha/ref`
	`89`	`+ exists(UsesStep head \|`
	`90`	`+ head.getCallee() = ["eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch"] and`
	`91`	`+ DataFlow::hasLocalFlowExpr(head, this.getArgumentExpr("ref"))`
	`92`	`+ )`
`83`	`93`	`or`
	`94`	`+ // heuristic base on the step id and field name`
`84`	`95`	`exists(StepsExpression e \|`
`85`	`96`	`this.getArgumentExpr("ref") = e and`
`86`	`97`	`(`
`87`	`98`	`e.getStepId().matches(["%sha%", "%head%", "branch"]) or`
`88`	`99`	`e.getFieldName().matches(["%sha%", "%head%", "branch"])`
`89`	`100`	`)`
`90`	`101`	`)`
`91`		`- or`
`92`		`- exists(UsesStep head \|`
`93`		`- head.getCallee() = ["eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch"] and`
`94`		`- DataFlow::hasLocalFlowExpr(head, this.getArgumentExpr("ref"))`
`95`		`- )`
`96`	`102`	`)`
`97`	`103`	`}`
`98`	`104`	`}`