Improve UntrustedCheckout query

Account for more events, more triggers and heuristics to detect git checkouts

Author: Alvaro Muñoz

ql/src/Security/CWE-829/UntrustedCheckout.ql

@@ -15,39 +15,68 @@
 
 import actions
 
-/**
- * An If node that contains an `actor` check
- */
-class ActorCheck extends If {
-  ActorCheck() {
+/** An If node that contains an actor, user or label check */
+class ControlCheck extends If {
+  ControlCheck() {
     this.getCondition().regexpMatch(".*github\\.(triggering_)?actor.*") or
-    this.getCondition().regexpMatch(".*github\\.event\\.pull_request\\.user\\.login.*")
+    this.getCondition().regexpMatch(".*github\\.event\\.pull_request\\.user\\.login.*") or
+    this.getCondition().regexpMatch(".*github\\.event\\.pull_request\\.labels.*") or
+    this.getCondition().regexpMatch(".*github\\.event\\.label\\.name.*")
   }
 }
 
-/**
- * An If node that contains a `label` check
- */
-class LabelCheck extends If {
-  LabelCheck() {
-    this.getCondition().regexpMatch(".*github\\.event\\.pull_request\\.labels.*") or
-    this.getCondition().regexpMatch(".*github\\.event\\.label\\.name.*")
+bindingset[s]
+predicate containsHeadRef(string s) {
+  s.matches("%" +
+      [
+        "github.event.number", // The pull request number.
+        "github.event.pull_request.head.ref", // The ref name of head.
+        "github.event.pull_request.head.sha", //  The commit SHA of head.
+        "github.event.pull_request.id", // The pull request ID.
+        "github.event.pull_request.number", // The pull request number.
+        "github.event.pull_request.merge_commit_sha", // The SHA of the merge commit.
+        "github.head_ref", // The head_ref or source branch of the pull request in a workflow run.
+        "github.event.workflow_run.head_branch", // The branch of the head commit.
+        "github.event.workflow_run.head_commit.id", // The SHA of the head commit.
+        "github.event.workflow_run.head_sha", // The SHA of the head commit.
+        "env.GITHUB_HEAD_REF",
+      ] + "%")
+}
+
+/** Checkout of a Pull Request HEAD ref */
+abstract class PRHeadCheckoutStep extends Step { }
+
+/** Checkout of a Pull Request HEAD ref using actions/checkout action */
+class ActionsCheckout extends PRHeadCheckoutStep instanceof UsesStep {
+  ActionsCheckout() {
+    this.getCallee() = "actions/checkout" and
+    containsHeadRef(this.getArgumentExpr("ref").getExpression())
+  }
+}
+
+/** Checkout of a Pull Request HEAD ref using git within a Run step */
+class GitCheckout extends PRHeadCheckoutStep instanceof Run {
+  GitCheckout() {
+    exists(string line |
+      this.getScript().splitAt("\n") = line and
+      line.regexpMatch(".*git\\s+fetch.*") and
+      (
+        containsHeadRef(line)
+        or
+        exists(string varname |
+          containsHeadRef(this.getInScopeEnvVarExpr(varname).getExpression()) and
+          line.matches("%" + varname + "%")
+        )
+      )
+    )
   }
 }
 
-from Workflow w, LocalJob job, UsesStep checkoutStep
+from Workflow w, PRHeadCheckoutStep checkout
 where
-  w.hasTriggerEvent("pull_request_target") and
-  w.getAJob() = job and
-  job.getAStep() = checkoutStep and
-  checkoutStep.getCallee() = "actions/checkout" and
-  checkoutStep
-      .getArgumentExpr("ref")
-      .getExpression()
-      .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
-  not exists(ActorCheck check | job.getIf() = check or checkoutStep.getIf() = check) and
-  not exists(LabelCheck check | job.getIf() = check or checkoutStep.getIf() = check)
-select checkoutStep, "Potential unsafe checkout of untrusted pull request on 'pull_request_target'."
+  w.hasTriggerEvent(["pull_request_target", "issue_comment", "workflow_run"]) and
+  w.getAJob().(LocalJob).getAStep() = checkout and
+  not exists(ControlCheck check |
+    checkout.getIf() = check or checkout.getEnclosingJob().getIf() = check
+  )
+select checkout, "Potential unsafe checkout of untrusted pull request on privileged workflow."

ql/test/query-tests/Security/CWE-829/.github/workflows/gitcheckout.yml

@@ -0,0 +1,23 @@
+on:
+  pull_request_target
+
+jobs:
+  build:
+    name: Build and test
+    runs-on: ubuntu-latest
+    steps:
+        # 1. Check out the content from an incoming pull request
+        - run: |
+            git fetch origin $HEAD_BRANCH
+            git checkout origin/master
+            git config user.name "release-hash-check"
+            git config user.email "<>"
+            git merge --no-commit --no-edit origin/$HEAD_BRANCH
+          env:
+            HEAD_BRANCH: ${{ github.head_ref }}
+        - uses: actions/setup-node@v1
+        # 2. Potentially untrusted commands are being run during "npm install" or "npm build" as
+        #    the build scripts and referenced packages are controlled by the author of the pull request
+        - run: |
+            npm install
+            npm build