feat(triggers): Add getEnclosingWorkflowStmt to Statement class

Author: Alvaro Muñoz

ql/lib/codeql/actions/Ast.qll

@@ -20,7 +20,10 @@ class AstNode instanceof YamlNode {
  * A statement is a group of expressions and/or statements that you design to carry out a task or an action.
  * Any statement that can return a value is automatically qualified to be used as an expression.
  */
-class Statement extends AstNode { }
+class Statement extends AstNode {
+  /** Gets the workflow that this job is a part of. */
+  WorkflowStmt getEnclosingWorkflowStmt() { exists(WorkflowStmt w | w.getAChildNode*() = result) }
+}
 
 /**
  * An expression is any word or group of words or symbols that is a value. In programming, an expression is a value, or anything that executes and ends up being a value.
@@ -53,6 +56,14 @@ class WorkflowStmt extends Statement instanceof Actions::Workflow {
   JobStmt getAJobStmt() { result = super.getJob(_) }
 
   JobStmt getJobStmt(string id) { result = super.getJob(id) }
+
+  predicate hasTriggerEvent(string trigger) {
+    exists(YamlNode n | n = super.getOn().(YamlMappingLikeNode).getNode(trigger))
+  }
+
+  string getATriggerEvent() {
+    exists(YamlNode n | n = super.getOn().(YamlMappingLikeNode).getNode(result))
+  }
 }
 
 class ReusableWorkflowStmt extends WorkflowStmt {
@@ -122,9 +133,6 @@ class JobStmt extends Statement instanceof Actions::Job {
    */
   string getId() { result = super.getId() }
 
-  /** Gets the workflow that this job is a part of. */
-  WorkflowStmt getWorkflowStmt() { result = super.getWorkflow() }
-
   /** Gets the step at the given index within this job. */
   StepStmt getStepStmt(int index) { result = super.getStep(index) }
 
@@ -222,7 +230,7 @@ class StepUsesExpr extends StepStmt, UsesExpr {
     )
     or
     exists(Actions::WorkflowEnv env |
-      env.getWorkflow() = this.getJobStmt().getWorkflowStmt() and
+      env.getWorkflow() = this.getJobStmt().getEnclosingWorkflowStmt() and
       env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
     )
   }
@@ -287,7 +295,7 @@ class JobUsesExpr extends UsesExpr instanceof YamlMapping {
     )
     or
     exists(Actions::WorkflowEnv env |
-      env.getWorkflow() = this.getJobStmt().getWorkflowStmt() and
+      env.getWorkflow() = this.getJobStmt().getEnclosingWorkflowStmt() and
       env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
     )
   }
@@ -320,7 +328,7 @@ class RunExpr extends StepStmt, Expression {
     )
     or
     exists(Actions::WorkflowEnv env |
-      env.getWorkflow() = this.getJobStmt().getWorkflowStmt() and
+      env.getWorkflow() = this.getJobStmt().getEnclosingWorkflowStmt() and
       env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
     )
   }

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

@@ -0,0 +1,47 @@
+/**
+ * @name Expression injection in Actions
+ * @description Using user-controlled GitHub Actions contexts like `run:` or `script:` may allow a malicious
+ *              user to inject code into the GitHub action.
+ * @kind path-problem
+ * @problem.severity error
+ * @security-severity 9
+ * @precision high
+ * @id actions/critical-expression-injection
+ * @tags actions
+ *       security
+ *       external/cwe/cwe-094
+ */
+
+import actions
+import codeql.actions.TaintTracking
+import codeql.actions.dataflow.FlowSources
+import codeql.actions.dataflow.ExternalFlow
+
+private class ExpressionInjectionSink extends DataFlow::Node {
+  ExpressionInjectionSink() {
+    exists(RunExpr e | e.getScriptExpr() = this.asExpr()) or
+    externallyDefinedSink(this, "expression-injection")
+  }
+}
+
+private module MyConfig implements DataFlow::ConfigSig {
+  predicate isSource(DataFlow::Node source) { source instanceof RemoteFlowSource }
+
+  predicate isSink(DataFlow::Node sink) { sink instanceof ExpressionInjectionSink }
+}
+
+module MyFlow = TaintTracking::Global<MyConfig>;
+
+import MyFlow::PathGraph
+
+from MyFlow::PathNode source, MyFlow::PathNode sink
+where
+  MyFlow::flowPath(source, sink) and
+  source
+      .getNode()
+      .asExpr()
+      .(Statement)
+      .getEnclosingWorkflowStmt()
+      .hasTriggerEvent("pull_request_target")
+select sink.getNode(), source, sink,
+  "Potential expression injection, which may be controlled by an external user."

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

@@ -4,7 +4,7 @@
  *              user to inject code into the GitHub action.
  * @kind path-problem
  * @problem.severity warning
- * @security-severity 9.3
+ * @security-severity 5.0
  * @precision high
  * @id actions/expression-injection
  * @tags actions

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

@@ -1,9 +1,6 @@
 name: CI
 
-on:
-  pull_request:
-    branches:
-      - main
+on: [pull_request_target, pull_request]
 
 jobs:
   changed_files: