Improve download artifact and untrusted checkout queries

Author: Alvaro Muñoz

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

@@ -16,19 +16,33 @@ abstract class UntrustedArtifactDownloadStep extends Step {
   abstract string getPath();
 }
 
+class GitHubDownloadArtifactActionStep extends UntrustedArtifactDownloadStep, UsesStep {
+  GitHubDownloadArtifactActionStep() {
+    // By default, the permissions are scoped so they can only download Artifacts within the current workflow run.
+    // To elevate permissions for this scenario, you can specify a github-token along with other repository and run identifiers
+    this.getCallee() = "actions/download-artifact" and
+    this.getArgument("run-id").matches("%github.event.workflow_run.id%") and
+    exists(this.getArgument("github-token"))
+  }
+
+  override string getPath() {
+    if exists(this.getArgument("path")) then result = this.getArgument("path") else result = ""
+  }
+}
+
 class DownloadArtifactActionStep extends UntrustedArtifactDownloadStep, UsesStep {
   DownloadArtifactActionStep() {
     this.getCallee() =
       [
-        "actions/download-artifact", "dawidd6/action-download-artifact",
-        "marcofaggian/action-download-multiple-artifacts", "benday-inc/download-latest-artifact",
-        "blablacar/action-download-last-artifact", "levonet/action-download-last-artifact",
-        "bettermarks/action-artifact-download", "aochmann/actions-download-artifact",
-        "cytopia/download-artifact-retry-action", "alextompkins/download-prior-artifact",
-        "nmerget/download-gzip-artifact", "benday-inc/download-artifact",
-        "synergy-au/download-workflow-artifacts-action", "ishworkh/docker-image-artifact-download",
-        "ishworkh/container-image-artifact-download", "sidx1024/action-download-artifact",
-        "hyperskill/azblob-download-artifact", "ma-ve/action-download-artifact-with-retry"
+        "dawidd6/action-download-artifact", "marcofaggian/action-download-multiple-artifacts",
+        "benday-inc/download-latest-artifact", "blablacar/action-download-last-artifact",
+        "levonet/action-download-last-artifact", "bettermarks/action-artifact-download",
+        "aochmann/actions-download-artifact", "cytopia/download-artifact-retry-action",
+        "alextompkins/download-prior-artifact", "nmerget/download-gzip-artifact",
+        "benday-inc/download-artifact", "synergy-au/download-workflow-artifacts-action",
+        "ishworkh/docker-image-artifact-download", "ishworkh/container-image-artifact-download",
+        "sidx1024/action-download-artifact", "hyperskill/azblob-download-artifact",
+        "ma-ve/action-download-artifact-with-retry"
       ] and
     (
       not exists(this.getArgument(["branch", "branch_name"])) or

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

@@ -93,7 +93,11 @@ class ActionsMutableRefCheckout extends MutableRefCheckoutStep instanceof UsesSt
       // 3rd party actions returning the PR head sha/ref
       exists(UsesStep step |
         (
-          step.getCallee() = ["eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch"] and
+          step.getCallee() =
+            [
+              "eficode/resolve-pr-refs", "xt0rted/pull-request-comment-branch",
+              "alessbell/pull-request-comment-branch", "gotson/pull-request-comment-branch"
+            ] and
           // TODO: This should be read step of the head_sha or head_ref output vars
           this.getArgument("ref").matches("%.head_ref%")
           or
@@ -229,10 +233,10 @@ 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.getIf() = this or
     step.getEnclosingJob().getIf() = this or
-    step.getEnclosingJob().getANeededJob().(LocalJob).getAStep().getIf() = this  or
-    step.getEnclosingJob().getANeededJob().(LocalJob).getIf() = this 
+    step.getEnclosingJob().getANeededJob().(LocalJob).getAStep().getIf() = this or
+    step.getEnclosingJob().getANeededJob().(LocalJob).getIf() = this
   }
 }
 
@@ -259,7 +263,7 @@ class ActorControlCheck extends ControlCheck {
           .regexpFind([
               "\\bgithub\\.actor\\b", "\\bgithub\\.triggering_actor\\b",
               "\\bgithub\\.event\\.comment\\.user\\.login\\b",
-              "\\bgithub\\.event\\.pull_request\\.user\\.login\\b", 
+              "\\bgithub\\.event\\.pull_request\\.user\\.login\\b",
             ], _, _)
     )
   }
@@ -270,10 +274,7 @@ class RepositoryControlCheck extends ControlCheck {
     // eg: github.repository == 'test/foo'
     exists(
       normalizeExpr(this.getCondition())
-          .regexpFind([
-              "\\bgithub\\.repository\\b",
-              "\\bgithub\\.repository_owner\\b",
-            ], _, _)
+          .regexpFind(["\\bgithub\\.repository\\b", "\\bgithub\\.repository_owner\\b",], _, _)
     )
   }
 }