From effa1e135670cfdaa01a83d42a156be6ff7eff87 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= <pwntester@github.com>
Date: Thu, 27 Jun 2024 22:53:20 +0200
Subject: [PATCH] Move ControlChecks to its own file

---
 .../codeql/actions/security/ControlChecks.qll | 65 +++++++++++++++++++
 .../security/UntrustedCheckoutQuery.qll       | 63 ------------------
 .../Security/CWE-285/ImproperAccessControl.ql |  1 +
 .../UntrustedCheckoutTOCTOUCritical.ql        |  1 +
 .../CWE-367/UntrustedCheckoutTOCTOUHigh.ql    |  1 +
 .../CWE-829/UntrustedCheckoutCritical.ql      |  1 +
 .../Security/CWE-829/UntrustedCheckoutHigh.ql |  1 +
 .../CWE-829/UntrustedCheckoutMedium.ql        |  1 +
 8 files changed, 71 insertions(+), 63 deletions(-)
 create mode 100644 ql/lib/codeql/actions/security/ControlChecks.qll

diff --git a/ql/lib/codeql/actions/security/ControlChecks.qll b/ql/lib/codeql/actions/security/ControlChecks.qll
new file mode 100644
index 00000000000..fdafda1fc27
--- /dev/null
+++ b/ql/lib/codeql/actions/security/ControlChecks.qll
@@ -0,0 +1,65 @@
+import actions
+
+/** An If node that contains an actor, user or label check */
+abstract class ControlCheck extends If {
+  predicate dominates(Step step) {
+    step.getIf() = this or
+    step.getEnclosingJob().getIf() = this or
+    step.getEnclosingJob().getANeededJob().(LocalJob).getAStep().getIf() = this or
+    step.getEnclosingJob().getANeededJob().(LocalJob).getIf() = this
+  }
+}
+
+class LabelControlCheck extends ControlCheck {
+  LabelControlCheck() {
+    // eg: contains(github.event.pull_request.labels.*.name, 'safe to test')
+    // eg: github.event.label.name == 'safe to test'
+    exists(
+      normalizeExpr(this.getCondition())
+          .regexpFind([
+              "\\bgithub\\.event\\.pull_request\\.labels\\b", "\\bgithub\\.event\\.label\\.name\\b"
+            ], _, _)
+    )
+  }
+}
+
+class ActorControlCheck extends ControlCheck {
+  ActorControlCheck() {
+    // eg: github.actor == 'dependabot[bot]'
+    // eg: github.triggering_actor == 'CI Agent'
+    // eg: github.event.pull_request.user.login == 'mybot'
+    exists(
+      normalizeExpr(this.getCondition())
+          .regexpFind([
+              "\\bgithub\\.actor\\b", "\\bgithub\\.triggering_actor\\b",
+              "\\bgithub\\.event\\.comment\\.user\\.login\\b",
+              "\\bgithub\\.event\\.pull_request\\.user\\.login\\b",
+            ], _, _)
+    )
+  }
+}
+
+class RepositoryControlCheck extends ControlCheck {
+  RepositoryControlCheck() {
+    // eg: github.repository == 'test/foo'
+    exists(
+      normalizeExpr(this.getCondition())
+          .regexpFind(["\\bgithub\\.repository\\b", "\\bgithub\\.repository_owner\\b",], _, _)
+    )
+  }
+}
+
+class AssociationControlCheck extends ControlCheck {
+  AssociationControlCheck() {
+    // eg: contains(fromJson('["MEMBER", "OWNER"]'), github.event.comment.author_association)
+    exists(
+      normalizeExpr(this.getCondition())
+          .regexpFind([
+              "\\bgithub\\.event\\.comment\\.author_association\\b",
+              "\\bgithub\\.event\\.issue\\.author_association\\b",
+              "\\bgithub\\.event\\.pull_request\\.author_association\\b",
+            ], _, _)
+    )
+  }
+}
+
diff --git a/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll b/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll
index 90b0a74d0ec..fcccc5d8a14 100644
--- a/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll
+++ b/ql/lib/codeql/actions/security/UntrustedCheckoutQuery.qll
@@ -233,66 +233,3 @@ class GhSHACheckout extends SHACheckoutStep instanceof Run {
     )
   }
 }
-
-/** An If node that contains an actor, user or label check */
-abstract class ControlCheck extends If {
-  predicate dominates(Step step) {
-    step.getIf() = this or
-    step.getEnclosingJob().getIf() = this or
-    step.getEnclosingJob().getANeededJob().(LocalJob).getAStep().getIf() = this or
-    step.getEnclosingJob().getANeededJob().(LocalJob).getIf() = this
-  }
-}
-
-class LabelControlCheck extends ControlCheck {
-  LabelControlCheck() {
-    // eg: contains(github.event.pull_request.labels.*.name, 'safe to test')
-    // eg: github.event.label.name == 'safe to test'
-    exists(
-      normalizeExpr(this.getCondition())
-          .regexpFind([
-              "\\bgithub\\.event\\.pull_request\\.labels\\b", "\\bgithub\\.event\\.label\\.name\\b"
-            ], _, _)
-    )
-  }
-}
-
-class ActorControlCheck extends ControlCheck {
-  ActorControlCheck() {
-    // eg: github.actor == 'dependabot[bot]'
-    // eg: github.triggering_actor == 'CI Agent'
-    // eg: github.event.pull_request.user.login == 'mybot'
-    exists(
-      normalizeExpr(this.getCondition())
-          .regexpFind([
-              "\\bgithub\\.actor\\b", "\\bgithub\\.triggering_actor\\b",
-              "\\bgithub\\.event\\.comment\\.user\\.login\\b",
-              "\\bgithub\\.event\\.pull_request\\.user\\.login\\b",
-            ], _, _)
-    )
-  }
-}
-
-class RepositoryControlCheck extends ControlCheck {
-  RepositoryControlCheck() {
-    // eg: github.repository == 'test/foo'
-    exists(
-      normalizeExpr(this.getCondition())
-          .regexpFind(["\\bgithub\\.repository\\b", "\\bgithub\\.repository_owner\\b",], _, _)
-    )
-  }
-}
-
-class AssociationControlCheck extends ControlCheck {
-  AssociationControlCheck() {
-    // eg: contains(fromJson('["MEMBER", "OWNER"]'), github.event.comment.author_association)
-    exists(
-      normalizeExpr(this.getCondition())
-          .regexpFind([
-              "\\bgithub\\.event\\.comment\\.author_association\\b",
-              "\\bgithub\\.event\\.issue\\.author_association\\b",
-              "\\bgithub\\.event\\.pull_request\\.author_association\\b",
-            ], _, _)
-    )
-  }
-}
diff --git a/ql/src/Security/CWE-285/ImproperAccessControl.ql b/ql/src/Security/CWE-285/ImproperAccessControl.ql
index 88ac3cee04d..16ae5c5fe9b 100644
--- a/ql/src/Security/CWE-285/ImproperAccessControl.ql
+++ b/ql/src/Security/CWE-285/ImproperAccessControl.ql
@@ -12,6 +12,7 @@
  */
 
 import codeql.actions.security.UntrustedCheckoutQuery
+import codeql.actions.security.ControlChecks
 
 from LocalJob job, LabelControlCheck check, MutableRefCheckoutStep checkout, Event event
 where
diff --git a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql
index ff9148ab583..3a049f67dea 100644
--- a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql
+++ b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUCritical.ql
@@ -14,6 +14,7 @@
 import actions
 import codeql.actions.security.UntrustedCheckoutQuery
 import codeql.actions.security.PoisonableSteps
+import codeql.actions.security.ControlChecks
 
 from ControlCheck check, MutableRefCheckoutStep checkout
 where
diff --git a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql
index ca1b855c6ec..b9a1e4c6301 100644
--- a/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql
+++ b/ql/src/Security/CWE-367/UntrustedCheckoutTOCTOUHigh.ql
@@ -14,6 +14,7 @@
 import actions
 import codeql.actions.security.UntrustedCheckoutQuery
 import codeql.actions.security.PoisonableSteps
+import codeql.actions.security.ControlChecks
 
 from ControlCheck check, MutableRefCheckoutStep checkout
 where
diff --git a/ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql b/ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql
index 9f7f3fd8cee..3a87b30be97 100644
--- a/ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql
+++ b/ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql
@@ -16,6 +16,7 @@
 import actions
 import codeql.actions.security.UntrustedCheckoutQuery
 import codeql.actions.security.PoisonableSteps
+import codeql.actions.security.ControlChecks
 
 query predicate edges(Step a, Step b) { a.getAFollowingStep() = b }
 
diff --git a/ql/src/Security/CWE-829/UntrustedCheckoutHigh.ql b/ql/src/Security/CWE-829/UntrustedCheckoutHigh.ql
index 980560dac9a..cb2f1cdaf95 100644
--- a/ql/src/Security/CWE-829/UntrustedCheckoutHigh.ql
+++ b/ql/src/Security/CWE-829/UntrustedCheckoutHigh.ql
@@ -16,6 +16,7 @@
 import actions
 import codeql.actions.security.UntrustedCheckoutQuery
 import codeql.actions.security.PoisonableSteps
+import codeql.actions.security.ControlChecks
 
 from LocalJob j, PRHeadCheckoutStep checkout
 where
diff --git a/ql/src/Security/CWE-829/UntrustedCheckoutMedium.ql b/ql/src/Security/CWE-829/UntrustedCheckoutMedium.ql
index 89d2e741306..3edde8dcf54 100644
--- a/ql/src/Security/CWE-829/UntrustedCheckoutMedium.ql
+++ b/ql/src/Security/CWE-829/UntrustedCheckoutMedium.ql
@@ -16,6 +16,7 @@
 import actions
 import codeql.actions.security.UntrustedCheckoutQuery
 import codeql.actions.security.PoisonableSteps
+import codeql.actions.security.ControlChecks
 
 from LocalJob j, PRHeadCheckoutStep checkout
 where