From 73fbd2311bc0eaded3f7855037c9249d252f6cf5 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Alvaro=20Mu=C3=B1oz?= <pwntester@github.com>
Date: Tue, 14 May 2024 10:20:04 +0200
Subject: [PATCH] Improper access check queries and tests

---
 .../Security/CWE-285/ImproperAccessControl.ql | 30 +++++++++++++++++++
 .../CWE-285/.github/workflows/test1.yml       | 20 +++++++++++++
 .../CWE-285/.github/workflows/test2.yml       | 20 +++++++++++++
 .../CWE-285/ImproperAccessControl.expected    |  1 +
 .../CWE-285/ImproperAccessControl.qlref       |  2 ++
 5 files changed, 73 insertions(+)
 create mode 100644 ql/src/Security/CWE-285/ImproperAccessControl.ql
 create mode 100644 ql/test/query-tests/Security/CWE-285/.github/workflows/test1.yml
 create mode 100644 ql/test/query-tests/Security/CWE-285/.github/workflows/test2.yml
 create mode 100644 ql/test/query-tests/Security/CWE-285/ImproperAccessControl.expected
 create mode 100644 ql/test/query-tests/Security/CWE-285/ImproperAccessControl.qlref

diff --git a/ql/src/Security/CWE-285/ImproperAccessControl.ql b/ql/src/Security/CWE-285/ImproperAccessControl.ql
new file mode 100644
index 00000000000..88ac3cee04d
--- /dev/null
+++ b/ql/src/Security/CWE-285/ImproperAccessControl.ql
@@ -0,0 +1,30 @@
+/**
+ * @name Improper Access Control
+ * @description The access control mechanism is not properly implemented, allowing untrusted code to be executed in a privileged context.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @security-severity 9.3
+ * @id actions/improper-access-control
+ * @tags actions
+ *       security
+ *       external/cwe/cwe-285
+ */
+
+import codeql.actions.security.UntrustedCheckoutQuery
+
+from LocalJob job, LabelControlCheck check, MutableRefCheckoutStep checkout, Event event
+where
+  job = checkout.getEnclosingJob() and
+  job.isPrivileged() and
+  job.getATriggerEvent() = event and
+  event.getName() = "pull_request_target" and
+  event.getAnActivityType() = "synchronize" and
+  job.getAStep() = checkout and
+  (
+    checkout.getIf() = check
+    or
+    checkout.getEnclosingJob().getIf() = check
+  )
+select checkout, "The checked-out code can be changed after the authorization check o step $@.",
+  check, check.toString()
diff --git a/ql/test/query-tests/Security/CWE-285/.github/workflows/test1.yml b/ql/test/query-tests/Security/CWE-285/.github/workflows/test1.yml
new file mode 100644
index 00000000000..48833460b44
--- /dev/null
+++ b/ql/test/query-tests/Security/CWE-285/.github/workflows/test1.yml
@@ -0,0 +1,20 @@
+name: Pull request feedback
+
+on:
+  pull_request_target:
+    types: [ opened, synchronize ]
+
+permissions: {}
+jobs:
+  test:
+    permissions:
+      contents: write
+      pull-requests: write
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout repo for OWNER TEST
+      uses: actions/checkout@v3
+      if: contains(github.event.pull_request.labels.*.name, 'safe to test')
+      with:
+        ref: ${{ github.event.pull_request.head.ref }}
+    - run: ./cmd
diff --git a/ql/test/query-tests/Security/CWE-285/.github/workflows/test2.yml b/ql/test/query-tests/Security/CWE-285/.github/workflows/test2.yml
new file mode 100644
index 00000000000..be6a6cf3939
--- /dev/null
+++ b/ql/test/query-tests/Security/CWE-285/.github/workflows/test2.yml
@@ -0,0 +1,20 @@
+name: Pull request feedback
+
+on:
+  pull_request_target:
+    types: [ labeled ]
+
+permissions: {}
+jobs:
+  test:
+    permissions:
+      contents: write
+      pull-requests: write
+    runs-on: ubuntu-latest
+    steps:
+    - name: Checkout repo for OWNER TEST
+      uses: actions/checkout@v3
+      if: contains(github.event.pull_request.labels.*.name, 'safe to test')
+      with:
+        ref: ${{ github.event.pull_request.head.ref }}
+    - run: ./cmd
diff --git a/ql/test/query-tests/Security/CWE-285/ImproperAccessControl.expected b/ql/test/query-tests/Security/CWE-285/ImproperAccessControl.expected
new file mode 100644
index 00000000000..53dd12b9fb6
--- /dev/null
+++ b/ql/test/query-tests/Security/CWE-285/ImproperAccessControl.expected
@@ -0,0 +1 @@
+| .github/workflows/test1.yml:15:7:20:4 | Uses Step | The checked-out code can be changed after the authorization check o step $@. | .github/workflows/test1.yml:17:11:17:75 | contain ...  test') | contain ...  test') |
diff --git a/ql/test/query-tests/Security/CWE-285/ImproperAccessControl.qlref b/ql/test/query-tests/Security/CWE-285/ImproperAccessControl.qlref
new file mode 100644
index 00000000000..09a19f21e3c
--- /dev/null
+++ b/ql/test/query-tests/Security/CWE-285/ImproperAccessControl.qlref
@@ -0,0 +1,2 @@
+Security/CWE-285/ImproperAccessControl.ql
+