Add Cache Poisoning Query

Author: Alvaro Muñoz

ql/lib/codeql/actions/security/CachePoisoningQuery.qll

@@ -0,0 +1,55 @@
+import actions
+
+abstract class CacheWritingStep extends Step { }
+
+class CacheActionUsesStep extends CacheWritingStep, UsesStep {
+  CacheActionUsesStep() { this.getCallee() = "actions/cache" }
+}
+
+class CacheActionSaveUsesStep extends CacheWritingStep, UsesStep {
+  CacheActionSaveUsesStep() { this.getCallee() = "actions/cache/save" }
+}
+
+class SetupJavaUsesStep extends CacheWritingStep, UsesStep {
+  SetupJavaUsesStep() {
+    this.getCallee() = "actions/setup-java" and
+    (
+      exists(this.getArgument("cache")) or
+      exists(this.getArgument("cache-dependency-path"))
+    )
+  }
+}
+
+class SetupGoUsesStep extends CacheWritingStep, UsesStep {
+  SetupGoUsesStep() { this.getCallee() = "actions/setup-go" }
+}
+
+class SetupNodeUsesStep extends CacheWritingStep, UsesStep {
+  SetupNodeUsesStep() {
+    this.getCallee() = "actions/setup-node" and
+    (
+      exists(this.getArgument("cache")) or
+      exists(this.getArgument("cache-dependency-path"))
+    )
+  }
+}
+
+class SetupPythonUsesStep extends CacheWritingStep, UsesStep {
+  SetupPythonUsesStep() {
+    this.getCallee() = "actions/setup-python" and
+    (
+      exists(this.getArgument("cache")) or
+      exists(this.getArgument("cache-dependency-path"))
+    )
+  }
+}
+
+class SetupDotnetUsesStep extends CacheWritingStep, UsesStep {
+  SetupDotnetUsesStep() {
+    this.getCallee() = "actions/setup-dotnet" and
+    (
+      exists(this.getArgument("cache")) or
+      exists(this.getArgument("cache-dependency-path"))
+    )
+  }
+}

ql/src/Security/CWE-349/CachePoisoning.ql

@@ -0,0 +1,36 @@
+/**
+ * @name Cache Poisoning
+ * @description The cache can be poisoned by untrusted code, leading to a cache poisoning attack.
+ * @kind problem
+ * @problem.severity error
+ * @precision high
+ * @security-severity 9.3
+ * @id actions/cache-poisoning
+ * @tags actions
+ *       security
+ *       external/cwe/cwe-349
+ */
+
+import actions
+import codeql.actions.security.UntrustedCheckoutQuery
+import codeql.actions.security.CachePoisoningQuery
+
+from Workflow w, PRHeadCheckoutStep checkout, LocalJob j
+where
+  // TODO: (require to collect trigger types)
+  // - add push to default branch?
+  // - exclude pull_request_target when branches_ignore includes default branch or when branches does not include the default branch
+  w.hasTriggerEvent([
+      "check_run", "check_suite", "delete", "discussion", "discussion_comment", "fork", "gollum",
+      "issue_comment", "issues", "label", "milestone", "project", "project_card", "project_column",
+      "public", "pull_request_comment", "pull_request_target", "repository_dispatch", "schedule",
+      "watch", "workflow_run"
+    ]) and
+  // Workflow is privileged
+  w.isPrivileged() and
+  // The workflow checkouts untrusted code from a pull request
+  j = w.getAJob() and
+  j.getAStep() = checkout and
+  // The checkout step is followed by a cache writing step
+  j.getAStep() instanceof CacheWritingStep
+select checkout, "Potential cache poisoning on privileged workflow."

ql/test/query-tests/Security/CWE-349/.github/workflows/test1.yml

@@ -0,0 +1,22 @@
+on:
+  issue_comment:
+    types: [created]
+
+jobs:
+  pr-comment:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: xt0rted/pull-request-comment-branch@v2
+        id: comment-branch
+
+      - uses: actions/checkout@v3
+        if: success()
+        with:
+          ref: ${{ steps.comment-branch.outputs.head_sha }}
+
+      - uses: actions/cache@v2
+        with:
+          path: ./poison
+          key: poison_key
+      - run: |
+          cat poison

ql/test/query-tests/Security/CWE-349/.github/workflows/test2.yml

@@ -0,0 +1,17 @@
+name: Cache Poisoning
+
+on: pull_request_target
+
+jobs:
+  poison:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+      - uses: actions/cache@v2
+        with:
+          path: ./poison
+          key: poison_key
+      - run: |
+          cat poison

ql/test/query-tests/Security/CWE-349/.github/workflows/test3.yml

@@ -0,0 +1,21 @@
+name: Cache Poisoning
+
+on: pull_request_target
+
+jobs:
+  poison:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+      - uses: actions/setup-java@v2
+        with:
+          distribution: 'zulu'
+          java-version: '21'
+          cache: 'gradle'
+          cache-dependency-path: |
+            sub-project/*.gradle*
+            sub-project/**/gradle-wrapper.properties
+      - run: |
+          java HelloWorldApp.java

ql/test/query-tests/Security/CWE-349/.github/workflows/test4.yml

@@ -0,0 +1,17 @@
+name: Cache Poisoning
+
+on: pull_request_target
+
+jobs:
+  poison:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+        with:
+          ref: ${{ github.event.pull_request.head.sha }}
+      - uses: actions/setup-java@v2
+        with:
+          distribution: 'zulu'
+          java-version: '21'
+      - run: |
+          java HelloWorldApp.java

ql/test/query-tests/Security/CWE-349/CachePoisoning.expected

@@ -0,0 +1,3 @@
+| .github/workflows/test1.yml:12:9:17:6 | Uses Step | Potential cache poisoning on privileged workflow. |
+| .github/workflows/test2.yml:9:9:12:6 | Uses Step | Potential cache poisoning on privileged workflow. |
+| .github/workflows/test3.yml:9:9:12:6 | Uses Step | Potential cache poisoning on privileged workflow. |

ql/test/query-tests/Security/CWE-349/CachePoisoning.qlref

@@ -0,0 +1,2 @@
+Security/CWE-349/CachePoisoning.ql
+