Merge pull request #20 from GitHubSecurityLab/untrusted_checkout

Author: Alvaro Muñoz

ql/lib/codeql/actions/Ast.qll

@@ -22,9 +22,7 @@ class AstNode instanceof YamlNode {
  */
 class Statement extends AstNode {
   /** Gets the workflow that this job is a part of. */
-  WorkflowStmt getEnclosingWorkflowStmt() {
-    this = result.getAChildNode*()
-  }
+  WorkflowStmt getEnclosingWorkflowStmt() { this = result.getAChildNode*() }
 }
 
 /**
@@ -174,6 +172,8 @@ class JobStmt extends Statement instanceof Actions::Job {
   predicate usesReusableWorkflow() {
     this.(YamlMapping).maps(any(YamlString s | s.getValue() = "uses"), _)
   }
+
+  IfStmt getIfStmt() { result = super.getIf() }
 }
 
 /**
@@ -183,6 +183,24 @@ class StepStmt extends Statement instanceof Actions::Step {
   string getId() { result = super.getId() }
 
   JobStmt getJobStmt() { result = super.getJob() }
+
+  IfStmt getIfStmt() { result = super.getIf() }
+}
+
+/**
+ * An If node representing a conditional statement.
+ */
+class IfStmt extends Statement {
+  YamlMapping parent;
+
+  IfStmt() {
+    (parent instanceof Actions::Step or parent instanceof Actions::Job) and
+    parent.lookup("if") = this
+  }
+
+  Statement getEnclosingStatement() { result = parent }
+
+  string getCondition() { result = this.(YamlScalar).getValue() }
 }
 
 /**

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

@@ -0,0 +1,47 @@
+/**
+ * @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 actions/untrusted-checkout
+ * @tags actions
+ *       security
+ *       external/cwe/cwe-094
+ */
+
+import actions
+
+/**
+ * An If node that contains an `actor` check
+ */
+class ActorCheckStmt extends IfStmt {
+  ActorCheckStmt() { this.getCondition().regexpMatch(".*github\\.(triggering_)?actor.*") }
+}
+
+/**
+ * An If node that contains a `label` check
+ */
+class LabelCheckStmt extends IfStmt {
+  LabelCheckStmt() { this.getCondition().regexpMatch(".*github\\.event\\.pull_request\\.labels.*") }
+}
+
+from WorkflowStmt w, JobStmt job, StepUsesExpr checkoutStep
+where
+  w.hasTriggerEvent("pull_request_target") and
+  w.getAJobStmt() = job and
+  job.getAStepStmt() = checkoutStep and
+  checkoutStep.getCallee() = "actions/checkout" and
+  checkoutStep
+      .getArgumentExpr("ref")
+      .(ExprAccessExpr)
+      .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(ActorCheckStmt check | job.getIfStmt() = check or checkoutStep.getIfStmt() = check) and
+  not exists(LabelCheckStmt check | job.getIfStmt() = check or checkoutStep.getIfStmt() = check)
+select checkoutStep, "Potential unsafe checkout of untrusted pull request on 'pull_request_target'."

ql/src/test/.github/workflows/actor_trusted_checkout.yml

@@ -0,0 +1,26 @@
+on:
+  pull_request_target
+
+jobs:
+  build:
+    name: Build and test
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+      if: ${{ github.actor == "admin" }}
+      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!

ql/src/test/.github/workflows/label_trusted_checkout.yml

@@ -0,0 +1,27 @@
+on:
+  pull_request_target:
+    types: [labeled]
+
+jobs:
+  build:
+    name: Build and test
+    runs-on: ubuntu-latest
+    if: contains(github.event.pull_request.labels.*.name, 'safe to test')
+    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!

ql/src/test/.github/workflows/untrusted_checkout.yml

@@ -0,0 +1,25 @@
+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!