[DIFF-INFORMED] Actions: CodeInjection

Query: https://github.com/d10c/codeql/blob/d10c/diff-informed-phase-3/actions/ql/src/Security/CWE-349/CachePoisoningViaCodeInjection.ql#L46

Author: d10c

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

@@ -3,6 +3,8 @@ private import codeql.actions.TaintTracking
 private import codeql.actions.dataflow.ExternalFlow
 import codeql.actions.dataflow.FlowSources
 import codeql.actions.DataFlow
+import codeql.actions.security.ControlChecks
+import codeql.actions.security.CachePoisoningQuery
 
 class CodeInjectionSink extends DataFlow::Node {
   CodeInjectionSink() {
@@ -11,6 +13,46 @@ class CodeInjectionSink extends DataFlow::Node {
   }
 }
 
+/**
+ * Get the relevant event for the sink in CodeInjectionCritical.ql.
+ */
+Event getRelevantCriticalEventForSink(DataFlow::Node sink) {
+  inPrivilegedContext(sink.asExpr(), result) and
+  not exists(ControlCheck check | check.protects(sink.asExpr(), result, "code-injection")) and
+  // exclude cases where the sink is a JS script and the expression uses toJson
+  not exists(UsesStep script |
+    script.getCallee() = "actions/github-script" and
+    script.getArgumentExpr("script") = sink.asExpr() and
+    exists(getAToJsonReferenceExpression(sink.asExpr().(Expression).getExpression(), _))
+  )
+}
+
+/**
+ * Get the relevant event for the sink in CachePoisoningViaCodeInjection.ql.
+ */
+Event getRelevantCachePoisoningEventForSink(DataFlow::Node sink) {
+  exists(LocalJob job |
+    job = sink.asExpr().getEnclosingJob() and
+    job.getATriggerEvent() = result and
+    // job can be triggered by an external user
+    result.isExternallyTriggerable() and
+    // excluding privileged workflows since they can be exploited in easier circumstances
+    // which is covered by `actions/code-injection/critical`
+    not job.isPrivilegedExternallyTriggerable(result) and
+    (
+      // the workflow runs in the context of the default branch
+      runsOnDefaultBranch(result)
+      or
+      // the workflow caller runs in the context of the default branch
+      result.getName() = "workflow_call" and
+      exists(ExternalJob caller |
+        caller.getCallee() = job.getLocation().getFile().getRelativePath() and
+        runsOnDefaultBranch(caller.getATriggerEvent())
+      )
+    )
+  )
+}
+
 /**
  * A taint-tracking configuration for unsafe user input
  * that is used to construct and evaluate a code script.
@@ -35,6 +77,18 @@ private module CodeInjectionConfig implements DataFlow::ConfigSig {
       exists(run.getScript().getAFileReadCommand())
     )
   }
+
+  predicate observeDiffInformedIncrementalMode() { any() }
+
+  Location getASelectedSourceLocation(DataFlow::Node source) { none() }
+
+  Location getASelectedSinkLocation(DataFlow::Node sink) {
+    result = sink.getLocation()
+    or
+    result = getRelevantCriticalEventForSink(sink).getLocation()
+    or
+    result = getRelevantCachePoisoningEventForSink(sink).getLocation()
+  }
 }
 
 /** Tracks flow of unsafe user input that is used to construct and evaluate a code script. */

actions/ql/src/Security/CWE-094/CodeInjectionCritical.ql

@@ -22,15 +22,8 @@ import codeql.actions.security.ControlChecks
 from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Event event
 where
   CodeInjectionFlow::flowPath(source, sink) and
-  inPrivilegedContext(sink.getNode().asExpr(), event) and
-  source.getNode().(RemoteFlowSource).getEventName() = event.getName() and
-  not exists(ControlCheck check | check.protects(sink.getNode().asExpr(), event, "code-injection")) and
-  // exclude cases where the sink is a JS script and the expression uses toJson
-  not exists(UsesStep script |
-    script.getCallee() = "actions/github-script" and
-    script.getArgumentExpr("script") = sink.getNode().asExpr() and
-    exists(getAToJsonReferenceExpression(sink.getNode().asExpr().(Expression).getExpression(), _))
-  )
+  event = getRelevantCriticalEventForSink(sink.getNode()) and
+  source.getNode().(RemoteFlowSource).getEventName() = event.getName()
 select sink.getNode(), source, sink,
   "Potential code injection in $@, which may be controlled by an external user ($@).", sink,
   sink.getNode().asExpr().(Expression).getRawExpression(), event, event.getName()

actions/ql/src/Security/CWE-349/CachePoisoningViaCodeInjection.ql

@@ -18,30 +18,13 @@ import codeql.actions.security.CachePoisoningQuery
 import CodeInjectionFlow::PathGraph
 import codeql.actions.security.ControlChecks
 
-from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, LocalJob job, Event event
+from CodeInjectionFlow::PathNode source, CodeInjectionFlow::PathNode sink, Event event
 where
   CodeInjectionFlow::flowPath(source, sink) and
-  job = sink.getNode().asExpr().getEnclosingJob() and
-  job.getATriggerEvent() = event and
-  // job can be triggered by an external user
-  event.isExternallyTriggerable() and
+  event = getRelevantCachePoisoningEventForSink(sink.getNode()) and
   // the checkout is not controlled by an access check
   not exists(ControlCheck check |
     check.protects(source.getNode().asExpr(), event, "code-injection")
-  ) and
-  // excluding privileged workflows since they can be exploited in easier circumstances
-  // which is covered by `actions/code-injection/critical`
-  not job.isPrivilegedExternallyTriggerable(event) and
-  (
-    // the workflow runs in the context of the default branch
-    runsOnDefaultBranch(event)
-    or
-    // the workflow caller runs in the context of the default branch
-    event.getName() = "workflow_call" and
-    exists(ExternalJob caller |
-      caller.getCallee() = job.getLocation().getFile().getRelativePath() and
-      runsOnDefaultBranch(caller.getATriggerEvent())
-    )
   )
 select sink.getNode(), source, sink,
   "Unprivileged code injection in $@, which may lead to cache poisoning ($@).", sink,