Merge pull request #9 from GitHubSecurityLab/content_set

feat(field-flow): enhance dataflow tracking

ql/lib/codeql/actions/Ast.qll

@@ -95,6 +95,8 @@ class OutputsStmt extends Statement instanceof YamlMapping {
     this.(YamlMapping).lookup(name).(YamlMapping).lookup("value") = result or
     this.(YamlMapping).lookup(name) = result
   }
+
+  string getAnOutputName() { this.(YamlMapping).maps(any(YamlString s | s.getValue() = result), _) }
 }
 
 class InputExpr extends Expression instanceof YamlString {
@@ -158,6 +160,10 @@ class JobStmt extends Statement instanceof Actions::Job {
    *     arg1: value1
    */
   JobUsesExpr getUsesExpr() { result.getJobStmt() = this }
+
+  predicate usesReusableWorkflow() {
+    this.(YamlMapping).maps(any(YamlString s | s.getValue() = "uses"), _)
+  }
 }
 
 /**
@@ -353,26 +359,51 @@ class ExprAccessExpr extends Expression instanceof YamlString {
   string getExpression() { result = expr }
 
   JobStmt getJobStmt() { result.getAChildNode*() = this }
+}
+
+/**
+ * A context access expression.
+ * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
+ */
+class CtxAccessExpr extends ExprAccessExpr {
+  CtxAccessExpr() {
+    expr.regexpMatch([
+        stepsCtxRegex(), needsCtxRegex(), jobsCtxRegex(), envCtxRegex(), inputsCtxRegex()
+      ])
+  }
+
+  abstract string getFieldName();
 
   abstract Expression getRefExpr();
 }
 
+private string stepsCtxRegex() { result = "steps\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)" }
+
+private string needsCtxRegex() { result = "needs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)" }
+
+private string jobsCtxRegex() { result = "jobs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)" }
+
+private string envCtxRegex() { result = "env\\.([A-Za-z0-9_-]+)" }
+
+private string inputsCtxRegex() { result = "inputs\\.([A-Za-z0-9_-]+)" }
+
 /**
- * Holds for an ExprAccessExpr accesing the `steps` context.
+ * Holds for an expression accesing the `steps` context.
  * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
  * e.g. `${{ steps.changed-files.outputs.all_changed_files }}`
  */
-class StepOutputAccessExpr extends ExprAccessExpr {
+class StepsCtxAccessExpr extends CtxAccessExpr {
   string stepId;
-  string varName;
+  string fieldName;
 
-  StepOutputAccessExpr() {
-    stepId =
-      this.getExpression().regexpCapture("steps\\.([A-Za-z0-9_-]+)\\.outputs\\.[A-Za-z0-9_-]+", 1) and
-    varName =
-      this.getExpression().regexpCapture("steps\\.[A-Za-z0-9_-]+\\.outputs\\.([A-Za-z0-9_-]+)", 1)
+  StepsCtxAccessExpr() {
+    expr.regexpMatch(stepsCtxRegex()) and
+    stepId = expr.regexpCapture(stepsCtxRegex(), 1) and
+    fieldName = expr.regexpCapture(stepsCtxRegex(), 2)
   }
 
+  override string getFieldName() { result = fieldName }
+
   override Expression getRefExpr() {
     this.getLocation().getFile() = result.getLocation().getFile() and
     result.(StepStmt).getId() = stepId
@@ -380,79 +411,112 @@ class StepOutputAccessExpr extends ExprAccessExpr {
 }
 
 /**
- * Holds for an ExprAccessExpr accesing the `needs` or `job` contexts.
+ * Holds for an expression accesing the `needs` context.
  * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
- * e.g. `${{ needs.job1.outputs.foo}}` or `${{ jobs.job1.outputs.foo}}` (for reusable workflows)
+ * e.g. `${{ needs.job1.outputs.foo}}`
  */
-class JobOutputAccessExpr extends ExprAccessExpr {
+class NeedsCtxAccessExpr extends CtxAccessExpr {
+  JobStmt job;
   string jobId;
-  string varName;
+  string fieldName;
 
-  JobOutputAccessExpr() {
-    jobId =
-      this.getExpression()
-          .regexpCapture("(needs|jobs)\\.([A-Za-z0-9_-]+)\\.outputs\\.[A-Za-z0-9_-]+", 2) and
-    varName =
-      this.getExpression()
-          .regexpCapture("(needs|jobs)\\.[A-Za-z0-9_-]+\\.outputs\\.([A-Za-z0-9_-]+)", 2)
+  NeedsCtxAccessExpr() {
+    expr.regexpMatch(needsCtxRegex()) and
+    jobId = expr.regexpCapture(needsCtxRegex(), 1) and
+    fieldName = expr.regexpCapture(needsCtxRegex(), 2) and
+    job.getId() = jobId
   }
 
+  predicate usesReusableWorkflow() { job.usesReusableWorkflow() }
+
+  override string getFieldName() { result = fieldName }
+
+  override Expression getRefExpr() {
+    job.getLocation().getFile() = this.getLocation().getFile() and
+    (
+      // regular jobs
+      job.getOutputStmt().getOutputExpr(fieldName) = result
+      or
+      // jobs calling reusable workflows
+      job.getUsesExpr() = result
+    )
+  }
+}
+
+/**
+ * Holds for an expression accesing the `jobs` context.
+ * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
+ * e.g. `${{ jobs.job1.outputs.foo}}` (within reusable workflows)
+ */
+class JobsCtxAccessExpr extends CtxAccessExpr {
+  string jobId;
+  string fieldName;
+
+  JobsCtxAccessExpr() {
+    expr.regexpMatch(jobsCtxRegex()) and
+    jobId = expr.regexpCapture(jobsCtxRegex(), 1) and
+    fieldName = expr.regexpCapture(jobsCtxRegex(), 2)
+  }
+
+  override string getFieldName() { result = fieldName }
+
   override Expression getRefExpr() {
     exists(JobStmt job |
       job.getId() = jobId and
       job.getLocation().getFile() = this.getLocation().getFile() and
-      (
-        // A Job can have multiple outputs, so we need to check both
-        // jobs.<job_id>.outputs.<output_name>
-        job.getOutputStmt().getOutputExpr(varName) = result
-        or
-        // jobs.<job_id>.uses (variables returned from the reusable workflow
-        job.getUsesExpr() = result
-      )
+      job.getOutputStmt().getOutputExpr(fieldName) = result
     )
   }
 }
 
 /**
- * Holds for an ExprAccessExpr accesing the `inputs` context.
+ * Holds for an expression the `inputs` context.
  * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
  * e.g. `${{ inputs.foo }}`
  */
-class InputAccessExpr extends ExprAccessExpr {
-  string paramName;
+class InputsCtxAccessExpr extends CtxAccessExpr {
+  string fieldName;
 
-  InputAccessExpr() {
-    paramName = this.getExpression().regexpCapture("inputs\\.([A-Za-z0-9_-]+)", 1)
+  InputsCtxAccessExpr() {
+    expr.regexpMatch(inputsCtxRegex()) and
+    fieldName = expr.regexpCapture(inputsCtxRegex(), 1)
   }
 
+  override string getFieldName() { result = fieldName }
+
   override Expression getRefExpr() {
     exists(ReusableWorkflowStmt w |
       w.getLocation().getFile() = this.getLocation().getFile() and
-      w.getInputsStmt().getInputExpr(paramName) = result
+      w.getInputsStmt().getInputExpr(fieldName) = result
     )
     or
     exists(CompositeActionStmt a |
       a.getLocation().getFile() = this.getLocation().getFile() and
-      a.getInputsStmt().getInputExpr(paramName) = result
+      a.getInputsStmt().getInputExpr(fieldName) = result
     )
   }
 }
 
 /**
- * Holds for an ExprAccessExpr accesing the `env` context.
+ * Holds for an expression accesing the `env` context.
  * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
  * e.g. `${{ env.foo }}`
  */
-class EnvAccessExpr extends ExprAccessExpr {
-  string varName;
+class EnvCtxAccessExpr extends CtxAccessExpr {
+  string fieldName;
 
-  EnvAccessExpr() { varName = this.getExpression().regexpCapture("env\\.([A-Za-z0-9_-]+)", 1) }
+  EnvCtxAccessExpr() {
+    expr.regexpMatch(envCtxRegex()) and
+    fieldName = expr.regexpCapture(envCtxRegex(), 1)
+  }
+
+  override string getFieldName() { result = fieldName }
 
   override Expression getRefExpr() {
-    exists(JobUsesExpr s | s.getEnvExpr(varName) = result)
+    exists(JobUsesExpr s | s.getEnvExpr(fieldName) = result)
     or
-    exists(StepUsesExpr s | s.getEnvExpr(varName) = result)
+    exists(StepUsesExpr s | s.getEnvExpr(fieldName) = result)
     or
-    exists(RunExpr s | s.getEnvExpr(varName) = result)
+    exists(RunExpr s | s.getEnvExpr(fieldName) = result)
   }
 }

ql/lib/codeql/actions/ast/internal/Actions.qll

@@ -294,8 +294,10 @@ module Actions {
     /** Gets the owner and name of the repository where the Action comes from, e.g. `actions/checkout` in `actions/checkout@v2`. */
     string getGitHubRepository() {
       result =
-        this.getValue().regexpCapture(usesParser(), 1) + "/" +
-          this.getValue().regexpCapture(usesParser(), 2)
+        (
+          this.getValue().regexpCapture(usesParser(), 1) + "/" +
+            this.getValue().regexpCapture(usesParser(), 2)
+        ).toLowerCase()
     }
 
     /** Gets the version reference used when checking out the Action, e.g. `v2` in `actions/checkout@v2`. */

ql/lib/codeql/actions/dataflow/ExternalFlow.qll

@@ -2,21 +2,31 @@ private import internal.ExternalFlowExtensions as Extensions
 import codeql.actions.DataFlow
 import actions
 
-/** Holds if a source model exists for the given parameters. */
+/**
+ * MaD sources
+ * Fields:
+ *    - action: Fully-qualified action name (NWO)
+ *    - version: Either '*' or a specific SHA/Tag
+ *    - output arg: To node (prefixed with either `env.` or `output.`)
+ *    - trigger: Triggering event under which this model introduces tainted data. Use `*` for any event.
+ */
 predicate sourceModel(string action, string version, string output, string trigger, string kind) {
   Extensions::sourceModel(action, version, output, trigger, kind)
 }
 
-/** Holds if a sink model exists for the given parameters. */
+/**
+ * MaD summaries
+ * Fields:
+ *    - action: Fully-qualified action name (NWO)
+ *    - version: Either '*' or a specific SHA/Tag
+ *    - input arg: From node (prefixed with either `env.` or `input.`)
+ *    - output arg: To node (prefixed with either `env.` or `output.`)
+ *    - kind: Either 'Taint' or 'Value'
+ */
 predicate summaryModel(string action, string version, string input, string output, string kind) {
   Extensions::summaryModel(action, version, input, output, kind)
 }
 
-/** Holds if a sink model exists for the given parameters. */
-predicate sinkModel(string action, string version, string input, string kind) {
-  Extensions::sinkModel(action, version, input, kind)
-}
-
 /**
  * MaD sinks
  * Fields:
@@ -25,7 +35,55 @@ predicate sinkModel(string action, string version, string input, string kind) {
  *    - input arg: sink node (prefixed with either `env.` or `input.`)
  *    - kind: sink kind
  */
-predicate sinkNode(DataFlow::ExprNode sink, string kind) {
+predicate sinkModel(string action, string version, string input, string kind) {
+  Extensions::sinkModel(action, version, input, kind)
+}
+
+predicate externallyDefinedSource(DataFlow::Node source, string sourceType, string fieldName) {
+  exists(UsesExpr uses, string action, string version, string trigger, string kind |
+    sourceModel(action, version, fieldName, trigger, kind) and
+    uses.getCallee() = action.toLowerCase() and
+    (
+      if version.trim() = "*"
+      then uses.getVersion() = any(string v)
+      else uses.getVersion() = version.trim()
+    ) and
+    (
+      if fieldName.trim().matches("env.%")
+      then source.asExpr() = uses.getEnvExpr(fieldName.trim().replaceAll("env\\.", ""))
+      else
+        if fieldName.trim().matches("output.%")
+        then
+          // 'output.' is the default qualifier
+          source.asExpr() = uses
+        else none()
+    ) and
+    sourceType = kind
+  )
+}
+
+predicate externallyDefinedSummary(DataFlow::Node pred, DataFlow::Node succ, DataFlow::ContentSet c) {
+  exists(UsesExpr uses, string action, string version, string input, string output |
+    c = any(DataFlow::FieldContent ct | ct.getName() = output.replaceAll("output\\.", "")) and
+    summaryModel(action, version, input, output, "taint") and
+    uses.getCallee() = action.toLowerCase() and
+    (
+      if version.trim() = "*"
+      then uses.getVersion() = any(string v)
+      else uses.getVersion() = version.trim()
+    ) and
+    (
+      if input.trim().matches("env.%")
+      then pred.asExpr() = uses.getEnvExpr(input.trim().replaceAll("env\\.", ""))
+      else
+        // 'input.' is the default qualifier
+        pred.asExpr() = uses.getArgumentExpr(input.trim().replaceAll("input\\.", ""))
+    ) and
+    succ.asExpr() = uses
+  )
+}
+
+predicate externallyDefinedSink(DataFlow::ExprNode sink, string kind) {
   exists(UsesExpr uses, string action, string version, string input |
     (
       if input.trim().matches("env.%")
@@ -33,7 +91,7 @@ predicate sinkNode(DataFlow::ExprNode sink, string kind) {
       else sink.asExpr() = uses.getArgumentExpr(input.trim())
     ) and
     sinkModel(action, version, input, kind) and
-    uses.getCallee() = action and
+    uses.getCallee() = action.toLowerCase() and
     (
       if version.trim() = "*"
       then uses.getVersion() = any(string v)

ql/lib/codeql/actions/dataflow/FlowSources.qll

@@ -126,40 +126,14 @@ private class EventSource extends RemoteFlowSource {
 }
 
 /**
- * MaD sources
- * Fields:
- *    - action: Fully-qualified action name (NWO)
- *    - version: Either '*' or a specific SHA/Tag
- *    - output arg: To node (prefixed with either `env.` or `output.`)
- *    - trigger: Triggering event under which this model introduces tainted data. Use `*` for any event.
+ * A Source of untrusted data defined in a MaD specification
  */
 private class ExternallyDefinedSource extends RemoteFlowSource {
-  string soutceType;
+  string sourceType;
 
-  ExternallyDefinedSource() {
-    exists(
-      UsesExpr uses, string action, string version, string output, string trigger, string kind
-    |
-      sourceModel(action, version, output, trigger, kind) and
-      uses.getCallee() = action and
-      (
-        if version.trim() = "*"
-        then uses.getVersion() = any(string v)
-        else uses.getVersion() = version.trim()
-      ) and
-      (
-        if output.trim().matches("env.%")
-        then this.asExpr() = uses.getEnvExpr(output.trim().replaceAll("output\\.", ""))
-        else
-          // 'output.' is the default qualifier
-          // TODO: Taint just the specified output
-          this.asExpr() = uses
-      ) and
-      soutceType = kind
-    )
-  }
+  ExternallyDefinedSource() { externallyDefinedSource(this, sourceType, _) }
 
-  override string getSourceType() { result = soutceType }
+  override string getSourceType() { result = sourceType }
 }
 
 /**

ql/lib/codeql/actions/dataflow/FlowSteps.qll

@@ -21,42 +21,11 @@ class AdditionalTaintStep extends Unit {
   abstract predicate step(DataFlow::Node node1, DataFlow::Node node2);
 }
 
-/**
- * MaD summaries
- * Fields:
- *    - action: Fully-qualified action name (NWO)
- *    - version: Either '*' or a specific SHA/Tag
- *    - input arg: From node (prefixed with either `env.` or `input.`)
- *    - output arg: To node (prefixed with either `env.` or `output.`)
- *    - kind: Either 'Taint' or 'Value'
- */
-predicate externallyDefinedSummary(DataFlow::Node pred, DataFlow::Node succ) {
-  exists(UsesExpr uses, string action, string version, string input |
-    // `output` not used yet
-    summaryModel(action, version, input, _, "taint") and
-    uses.getCallee() = action and
-    (
-      if version.trim() = "*"
-      then uses.getVersion() = any(string v)
-      else uses.getVersion() = version.trim()
-    ) and
-    (
-      if input.trim().matches("env.%")
-      then pred.asExpr() = uses.getEnvExpr(input.trim().replaceAll("env\\.", ""))
-      else
-        // 'input.' is the default qualifier
-        pred.asExpr() = uses.getArgumentExpr(input.trim().replaceAll("input\\.", ""))
-    ) and
-    succ.asExpr() = uses
-  )
-}
-
-private class ExternallyDefinedSummary extends AdditionalTaintStep {
-  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    externallyDefinedSummary(pred, succ)
-  }
-}
-
+// private class RunEnvToScriptStep extends AdditionalTaintStep {
+//   override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
+//     runEnvToScriptstep(pred, succ)
+//   }
+// }
 /**
  * Holds if a Run step declares an environment variable, uses it in its script and sets an output in its script.
  * e.g.
@@ -68,23 +37,21 @@ private class ExternallyDefinedSummary extends AdditionalTaintStep {
  *      INITIAL_URL=$(echo "$BODY" | grep -o 'https://github.com/github/release-assets/assets/[^ >]*')
  *      echo "Cleaned Initial URL: $INITIAL_URL"
  *      echo "::set-output name=initial_url::$INITIAL_URL"
+ *      echo "foo=$(echo $TAINTED)" >> $GITHUB_OUTPUT
+ *      echo "test=${{steps.step1.outputs.MSG}}" >> "$GITHUB_OUTPUT"
  */
-private class RunEnvToScriptStep extends AdditionalTaintStep {
-  override predicate step(DataFlow::Node pred, DataFlow::Node succ) {
-    runEnvToScriptstep(pred, succ)
-  }
-}
-
-predicate runEnvToScriptstep(DataFlow::Node pred, DataFlow::Node succ) {
-  exists(RunExpr r, string varName |
+predicate runEnvToScriptstep(DataFlow::Node pred, DataFlow::Node succ, DataFlow::ContentSet c) {
+  exists(RunExpr r, string varName, string output |
+    c = any(DataFlow::FieldContent ct | ct.getName() = output.replaceAll("output\\.", "")) and
     r.getEnvExpr(varName) = pred.asExpr() and
     exists(string script, string line |
       script = r.getScript() and
       line = script.splitAt("\n") and
       (
-        line.regexpMatch(".*::set-output\\s+name.*") or
-        line.regexpMatch(".*>>\\s*\\$GITHUB_OUTPUT.*")
+        output = line.regexpCapture(".*::set-output\\s+name=(.*)::.*", 1) or
+        output = line.regexpCapture(".*echo\\s*\"(.*)=.*\\s*>>\\s*(\")?\\$GITHUB_OUTPUT.*", 1)
       ) and
+      // TODO: repalce script with line below
       script.indexOf("$" + ["", "{", "ENV{"] + varName) > 0
     ) and
     succ.asExpr() = r

ql/lib/codeql/actions/dataflow/internal/DataFlowPrivate.qll

@@ -4,6 +4,8 @@ private import codeql.actions.Cfg as Cfg
 private import codeql.Locations
 private import codeql.actions.controlflow.BasicBlocks
 private import DataFlowPublic
+private import codeql.actions.dataflow.ExternalFlow
+private import codeql.actions.dataflow.FlowSteps
 
 cached
 newtype TNode = TExprNode(DataFlowExpr e)
@@ -129,25 +131,43 @@ predicate compatibleTypes(DataFlowType t1, DataFlowType t2) { t1 = t2 }
 
 predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) { none() }
 
-private newtype TContent = TNoContent() { none() }
+newtype TContent =
+  TFieldContent(string name) {
+    name = any(StepsCtxAccessExpr a).getFieldName() or
+    name = any(NeedsCtxAccessExpr a).getFieldName() or
+    name = any(JobsCtxAccessExpr a).getFieldName()
+  }
 
+/**
+ * A reference contained in an object. Examples include instance fields, the
+ * contents of a collection object, the contents of an array or pointer.
+ */
 class Content extends TContent {
+  /** Gets the type of the contained data for the purpose of type pruning. */
+  DataFlowType getType() { any() }
+
   /** Gets a textual representation of this element. */
-  string toString() { none() }
+  abstract string toString();
+
+  /**
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+   */
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    filepath = "" and startline = 0 and startcolumn = 0 and endline = 0 and endcolumn = 0
+  }
 }
 
-predicate forceHighPrecision(Content c) { none() }
+predicate forceHighPrecision(Content c) { c instanceof FieldContent }
 
-newtype TContentSet = TNoContentSet() { none() }
+class ContentApprox = ContentSet;
 
-private newtype TContentApprox = TNoContentApprox() { none() }
-
-class ContentApprox extends TContentApprox {
-  /** Gets a textual representation of this element. */
-  string toString() { none() }
-}
-
-ContentApprox getContentApprox(Content c) { none() }
+ContentApprox getContentApprox(Content c) { result = c }
 
 /**
  * Made a string to match the ArgumentPosition type.
@@ -169,11 +189,15 @@ predicate parameterMatch(ParameterPosition ppos, ArgumentPosition apos) { ppos =
 
 /**
  * Holds if there is a local flow step between a ${{}} expression accesing a step output variable and the step output itself
+ * But only for those cases where the step output is defined externally in a MaD specification.
+ * The reason for this is that we don't currently have a way to specify that a source starts with a non-empty access
+ * path so  the easiest thing is to add the corresponding read steps of that field as local flow steps as well.
  * e.g. ${{ steps.step1.output.foo }}
  */
 predicate stepsCtxLocalStep(Node nodeFrom, Node nodeTo) {
-  exists(StepStmt astFrom, StepOutputAccessExpr astTo |
-    (astFrom instanceof UsesExpr or astFrom instanceof RunExpr) and
+  exists(StepStmt astFrom, StepsCtxAccessExpr astTo |
+    externallyDefinedSource(nodeFrom, _, "output." + astTo.getFieldName()) and
+    astFrom instanceof UsesExpr and
     astFrom = nodeFrom.asExpr() and
     astTo = nodeTo.asExpr() and
     astTo.getRefExpr() = astFrom
@@ -182,13 +206,14 @@ predicate stepsCtxLocalStep(Node nodeFrom, Node nodeTo) {
 
 /**
  * Holds if there is a local flow step between a ${{}} expression accesing a job output variable and the job output itself
- * e.g. ${{ needs.job1.output.foo }} or ${{ job.job1.output.foo }}
+ * e.g. ${{ needs.job1.output.foo }} or ${{ jobs.job1.output.foo }}
  */
 predicate jobsCtxLocalStep(Node nodeFrom, Node nodeTo) {
-  exists(Expression astFrom, JobOutputAccessExpr astTo |
+  exists(Expression astFrom, CtxAccessExpr astTo |
     astFrom = nodeFrom.asExpr() and
     astTo = nodeTo.asExpr() and
-    astTo.getRefExpr() = astFrom
+    astTo.getRefExpr() = astFrom and
+    (astTo instanceof NeedsCtxAccessExpr or astTo instanceof JobsCtxAccessExpr)
   )
 }
 
@@ -197,7 +222,7 @@ predicate jobsCtxLocalStep(Node nodeFrom, Node nodeTo) {
  * e.g. ${{ inputs.foo }}
  */
 predicate inputsCtxLocalStep(Node nodeFrom, Node nodeTo) {
-  exists(Expression astFrom, InputAccessExpr astTo |
+  exists(Expression astFrom, InputsCtxAccessExpr astTo |
     astFrom = nodeFrom.asExpr() and
     astTo = nodeTo.asExpr() and
     astTo.getRefExpr() = astFrom
@@ -209,10 +234,13 @@ predicate inputsCtxLocalStep(Node nodeFrom, Node nodeTo) {
  * e.g. ${{ env.foo }}
  */
 predicate envCtxLocalStep(Node nodeFrom, Node nodeTo) {
-  exists(Expression astFrom, EnvAccessExpr astTo |
+  exists(Expression astFrom, EnvCtxAccessExpr astTo |
     astFrom = nodeFrom.asExpr() and
     astTo = nodeTo.asExpr() and
-    astTo.getRefExpr() = astFrom
+    (
+      externallyDefinedSource(nodeFrom, _, "env." + astTo.getFieldName()) or
+      astTo.getRefExpr() = astFrom
+    )
   )
 }
 
@@ -244,19 +272,63 @@ predicate simpleLocalFlowStep(Node nodeFrom, Node nodeTo) { localFlowStep(nodeFr
  */
 predicate jumpStep(Node nodeFrom, Node nodeTo) { none() }
 
+/**
+ * A read step to read the value of a ReusableWork uses step  and connect it to its
+ * corresponding JobOutputAccessExpr
+ */
+predicate reusableWorkflowReturnReadStep(Node node1, Node node2, ContentSet c) {
+  exists(NeedsCtxAccessExpr expr, string fieldName |
+    expr.usesReusableWorkflow() and
+    expr.getRefExpr() = node1.asExpr() and
+    expr.getFieldName() = fieldName and
+    expr = node2.asExpr() and
+    c = any(FieldContent ct | ct.getName() = fieldName)
+  )
+}
+
 /**
  * Holds if data can flow from `node1` to `node2` via a read of `c`.  Thus,
  * `node1` references an object with a content `c.getAReadContent()` whose
  * value ends up in `node2`.
  */
-predicate readStep(Node node1, ContentSet c, Node node2) { none() }
+predicate readStep(Node node1, ContentSet c, Node node2) {
+  // TODO: Extract to its own predicate
+  exists(StepsCtxAccessExpr access |
+    c = any(FieldContent ct | ct.getName() = access.getFieldName()) and
+    node1.asExpr() = access.getRefExpr() and
+    node2.asExpr() = access
+  )
+  or
+  reusableWorkflowReturnReadStep(node1, node2, c)
+}
+
+/**
+ * A store step to store the value of a ReusableWorkflowStmt output expr into the return node (node2)
+ * with a given access path (fieldName)
+ */
+predicate reusableWorkflowReturnStoreStep(Node node1, Node node2, ContentSet c) {
+  exists(ReusableWorkflowStmt stmt, OutputsStmt out, string fieldName |
+    out = stmt.getOutputsStmt() and
+    node1.asExpr() = out.getOutputExpr(fieldName) and
+    node2.asExpr() = out and
+    c = any(FieldContent ct | ct.getName() = fieldName)
+  )
+}
 
 /**
  * Holds if data can flow from `node1` to `node2` via a store into `c`.  Thus,
  * `node2` references an object with a content `c.getAStoreContent()` that
  * contains the value of `node1`.
  */
-predicate storeStep(Node node1, ContentSet c, Node node2) { none() }
+predicate storeStep(Node node1, ContentSet c, Node node2) {
+  reusableWorkflowReturnStoreStep(node1, node2, c)
+  or
+  // TODO: rename to xxxxStoreStep
+  externallyDefinedSummary(node1, node2, c)
+  or
+  // TODO: rename to xxxxStoreStep
+  runEnvToScriptstep(node1, node2, c)
+}
 
 /**
  * Holds if values stored inside content `c` are cleared at node `n`. For example,

ql/lib/codeql/actions/dataflow/internal/DataFlowPublic.qll

@@ -83,18 +83,18 @@ class ArgumentNode extends ExprNode {
  * Reusable workflow output nodes
  */
 class ReturnNode extends ExprNode {
-  private OutputExpr output;
+  private OutputsStmt outputs;
 
   ReturnNode() {
-    this.asExpr() = output and
-    output = any(OutputsStmt s).getOutputExpr(_)
+    this.asExpr() = outputs and
+    outputs = any(ReusableWorkflowStmt s).getOutputsStmt()
   }
 
   ReturnKind getKind() { result = TNormalReturn() }
 
-  override string toString() { result = "output " + output.toString() }
+  override string toString() { result = "output " + outputs.toString() }
 
-  override Location getLocation() { result = output.getLocation() }
+  override Location getLocation() { result = outputs.getLocation() }
 }
 
 /** Gets the node corresponding to `e`. */
@@ -106,13 +106,38 @@ Node exprNode(DataFlowExpr e) { result = TExprNode(e) }
  * The set may be interpreted differently depending on whether it is
  * stored into (`getAStoreContent`) or read from (`getAReadContent`).
  */
-class ContentSet extends TContentSet {
-  /** Gets a textual representation of this element. */
-  string toString() { none() }
-
+class ContentSet instanceof Content {
   /** Gets a content that may be stored into when storing into this set. */
-  Content getAStoreContent() { none() }
+  Content getAStoreContent() { result = this }
 
   /** Gets a content that may be read from when reading from this set. */
-  Content getAReadContent() { none() }
+  Content getAReadContent() { result = this }
+
+  /** Gets a textual representation of this content set. */
+  string toString() { result = super.toString() }
+
+  /**
+   * Holds if this element is at the specified location.
+   * The location spans column `startcolumn` of line `startline` to
+   * column `endcolumn` of line `endline` in file `filepath`.
+   * For more information, see
+   * [Locations](https://codeql.github.com/docs/writing-codeql-queries/providing-locations-in-codeql-queries/).
+   */
+  predicate hasLocationInfo(
+    string filepath, int startline, int startcolumn, int endline, int endcolumn
+  ) {
+    super.hasLocationInfo(filepath, startline, startcolumn, endline, endcolumn)
+  }
+}
+
+/** A field of an object, for example an instance variable. */
+class FieldContent extends Content, TFieldContent {
+  private string name;
+
+  FieldContent() { this = TFieldContent(name) }
+
+  /** Gets the name of the field. */
+  string getName() { result = name }
+
+  override string toString() { result = name }
 }

ql/lib/ext/tj-actions-changed-files.model.yml

@@ -3,26 +3,26 @@ extensions:
       pack: codeql/actions-all
       extensible: sourceModel
     data:
-      - [ "tj-actions/changed-files", "*", "added_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "all_changed_and_modified_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "all_changed_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "all_modified_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "all_old_new_renamed_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "any_changed", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "any_deleted", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "any_modified", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "changed_keys", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "copied_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "deleted_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "modified_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "modified_keys", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "only_changed", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "only_deleted", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "only_modified", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "other_changed_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "other_deleted_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "other_modified_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "renamed_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "type_changed_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "unknown_files", "*", "PR changed files" ]
-      - [ "tj-actions/changed-files", "*", "unmerged_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.added_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.all_changed_and_modified_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.all_changed_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.all_modified_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.all_old_new_renamed_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.any_changed", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.any_deleted", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.any_modified", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.changed_keys", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.copied_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.deleted_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.modified_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.modified_keys", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.only_changed", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.only_deleted", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.only_modified", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.other_changed_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.other_deleted_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.other_modified_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.renamed_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.type_changed_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.unknown_files", "*", "PR changed files" ]
+      - [ "tj-actions/changed-files", "*", "output.unmerged_files", "*", "PR changed files" ]

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

@@ -20,7 +20,7 @@ import codeql.actions.dataflow.ExternalFlow
 private class ExpressionInjectionSink extends DataFlow::Node {
   ExpressionInjectionSink() {
     exists(RunExpr e | e.getScriptExpr() = this.asExpr()) or
-    sinkNode(this, "expression-injection")
+    externallyDefinedSink(this, "expression-injection")
   }
 }
 
@@ -37,5 +37,5 @@ import MyFlow::PathGraph
 from MyFlow::PathNode source, MyFlow::PathNode sink
 where MyFlow::flowPath(source, sink)
 select sink.getNode(), source, sink,
-  "Potential injection from the ${{ " + sink.getNode().asExpr().(ExprAccessExpr).getExpression() +
+  "Potential injection from the ${{ " + sink.getNode().asExpr().(CtxAccessExpr).getExpression() +
     " }}, which may be controlled by an external user."

ql/src/test/.github/workflows/ci-cleanup.yml

@@ -1,47 +0,0 @@
-run-name: Cleanup ${{ github.head_ref }}
-on:
-  pull_request_target:
-    types: labeled
-    paths:
-      - "images/**"
-
-jobs:
-  clean_ci:
-    name: Clean CI runs
-    runs-on: ubuntu-latest
-    permissions:
-      actions: write
-    steps:
-      - env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-        shell: pwsh
-        run: |
-          $startDate = Get-Date -UFormat %s
-          $workflows = @("macos11", "macos12", "ubuntu2004", "ubuntu2204", "windows2019", "windows2022")
-          while ($true) {
-            $continue = $false
-            foreach ($wf in $workflows) {
-              $skippedCommand = "gh run list --workflow ${wf}.yml --branch ${{ github.event.pull_request.head.ref }} --repo ${{ github.repository }} --status skipped --json databaseId"
-              $skippedIds = Invoke-Expression -Command $skippedCommand | ConvertFrom-Json | ForEach-Object { $_.databaseId }
-              $skippedIds | ForEach-Object {
-                $deleteCommand = "gh run delete --repo ${{ github.repository }} $_"
-                Invoke-Expression -Command $deleteCommand
-              }
-              $pendingCommand = "gh run list --workflow ${wf}.yml --branch ${{ github.event.pull_request.head.ref }} --repo ${{ github.repository }} --status requested --json databaseId --template '{{ . | len }}'"
-              $pending = Invoke-Expression -Command $pendingCommand
-              if ($pending -gt 0) {
-                Write-Host "Pending for ${wf}.yml: $pending run(s)"
-                $continue = $true
-              }
-            }
-            if ($continue -eq $false) {
-              Write-Host "All done, exiting"
-              break
-            }
-            $curDate = Get-Date -UFormat %s
-            if (($curDate - $startDate) -gt 60) {
-              Write-Host "Reached timeout, exiting"
-              break
-            }
-            Write-Host "Waiting 5 seconds..."
-            Start-Sleep -Seconds 5

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

@@ -14,35 +14,39 @@ jobs:
 
       - name: Extract and Clean Initial URL
         id: extract-url
+        env:
+          BODY: ${{ github.event.comment.body }}
         run: |
-          INITIAL_URL=$(echo "${{ github.event.comment.body }}" | grep -o 'https://github.com/github/release-assets/assets/[^ >]*')
+          INITIAL_URL=$(echo "$BODY" | grep -o 'https://github.com/github/release-assets/assets/[^ >]*')
           echo "Cleaned Initial URL: $INITIAL_URL"
           echo "::set-output name=initial_url::$INITIAL_URL"
 
       - name: Get Redirected URL with Debugging
         id: curl
+        env:
+          INITIAL_URL: ${{ steps.extract-url.outputs.initial_url }}
         run: |
-          REDIRECTED_URL=$(curl -L -o /dev/null -w %{url_effective} -sS "${{ steps.extract-url.outputs.initial_url }}")
+          REDIRECTED_URL=$(curl -L -o /dev/null -w %{url_effective} -sS "$INITIAL_URL")
           echo "Curl Command Executed"
           echo "Redirected URL: $REDIRECTED_URL"
           echo "::set-output name=redirected_url::$REDIRECTED_URL"
 
       - name: Trim URL after PNG
         id: trim-url
+        env:
+          REDIRECTED_URL: ${{ steps.curl.outputs.redirected_url }}
         run: |
-          TRIMMED_URL=$(echo "${{ steps.curl.outputs.redirected_url }}" | sed 's/\(.*\.png\).*/\1/')
+          TRIMMED_URL=$(echo "$REDIRECTED_URL" | sed 's/\(.*\.png\).*/\1/')
           echo "Trimmed URL: $TRIMMED_URL"
           echo "::set-output name=trimmed_url::$TRIMMED_URL"
 
-      - name: Output Final Trimmed URL
-        run: |
-          echo "Final Trimmed Image URL: ${{ steps.trim-url.outputs.trimmed_url }}"
-
       - name: Update Comment with New URL
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
+          COMMENT_URL: ${{ github.event.comment.url }}
+          ORIGINAL_COMMENT_BODY: ${{ github.event.comment.body }}
         run: |
-          COMMENT_URL="${{ github.event.comment.url }}"
           NEW_COMMENT_BODY="Use this link to include this asset in your changelog: ${{ steps.trim-url.outputs.trimmed_url }}"
-          ORIGINAL_COMMENT_BODY="${{ github.event.comment.body }}"
           UPDATED_COMMENT="${ORIGINAL_COMMENT_BODY} 👀 ${NEW_COMMENT_BODY}"
 
           PAYLOAD=$(jq -n --arg body "$UPDATED_COMMENT" '{"body": $body}')
@@ -51,5 +55,3 @@ jobs:
                -H "Accept: application/vnd.github.v3+json" \
                "${COMMENT_URL}" \
                -d "$PAYLOAD"
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}

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

@@ -1,61 +0,0 @@
-name: Image URL Processing
-
-on:
-  issue_comment:
-    types: [created]
-
-jobs:
-  process-image-url:
-    runs-on: ubuntu-latest
-    if: contains(github.event.comment.body, 'https://github.com/github/release-assets/assets/')
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v2
-
-      - name: Extract and Clean Initial URL
-        id: extract-url
-        env:
-          BODY: ${{ github.event.comment.body }}
-        run: |
-          INITIAL_URL=$(echo "$BODY" | grep -o 'https://github.com/github/release-assets/assets/[^ >]*')
-          echo "Cleaned Initial URL: $INITIAL_URL"
-          echo "::set-output name=initial_url::$INITIAL_URL"
-
-      - name: Get Redirected URL with Debugging
-        id: curl
-        env:
-          INITIAL_URL: ${{ steps.extract-url.outputs.initial_url }}
-        run: |
-          REDIRECTED_URL=$(curl -L -o /dev/null -w %{url_effective} -sS "$INITIAL_URL")
-          echo "Curl Command Executed"
-          echo "Redirected URL: $REDIRECTED_URL"
-          echo "::set-output name=redirected_url::$REDIRECTED_URL"
-
-      - name: Trim URL after PNG
-        id: trim-url
-        env:
-          REDIRECTED_URL: ${{ steps.curl.outputs.redirected_url }}
-        run: |
-          TRIMMED_URL=$(echo "$REDIRECTED_URL" | sed 's/\(.*\.png\).*/\1/')
-          echo "Trimmed URL: $TRIMMED_URL"
-          echo "::set-output name=trimmed_url::$TRIMMED_URL"
-
-      - name: Output Final Trimmed URL
-        run: |
-          echo "Final Trimmed Image URL: ${{ steps.trim-url.outputs.trimmed_url }}"
-
-      - name: Update Comment with New URL
-        env:
-          GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
-          COMMENT_URL: ${{ github.event.comment.url }}
-          ORIGINAL_COMMENT_BODY: ${{ github.event.comment.body }}
-        run: |
-          NEW_COMMENT_BODY="Use this link to include this asset in your changelog: ${{ steps.trim-url.outputs.trimmed_url }}"
-          UPDATED_COMMENT="${ORIGINAL_COMMENT_BODY} 👀 ${NEW_COMMENT_BODY}"
-
-          PAYLOAD=$(jq -n --arg body "$UPDATED_COMMENT" '{"body": $body}')
-          curl -X PATCH \
-               -H "Authorization: token ${{ secrets.GITHUB_TOKEN }}" \
-               -H "Accept: application/vnd.github.v3+json" \
-               "${COMMENT_URL}" \
-               -d "$PAYLOAD"

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

@@ -1,27 +0,0 @@
-name: Image URL Processing
-
-on:
-  issue_comment:
-    types: [created]
-
-jobs:
-  process-image-url:
-    runs-on: ubuntu-latest
-    if: contains(github.event.comment.body, 'https://github.com/github/release-assets/assets/')
-    steps:
-      - name: Checkout
-        uses: actions/checkout@v2
-
-      - name: Extract and Clean Initial URL
-        id: source
-        env:
-          BODY: ${{ github.event.comment.body }}
-        run: |
-          INITIAL_URL=$(echo "$BODY" | grep -o 'https://github.com/github/release-assets/assets/[^ >]*')
-          echo "Cleaned Initial URL: $INITIAL_URL"
-          echo "::set-output name=initial_url::$INITIAL_URL"
-
-      - name: Get Redirected URL with Debugging
-        id: sink
-        run: |
-          echo ${{ steps.source.outputs.initial_url }}

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

@@ -5,12 +5,15 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - id: source 
+      - id: summary
         uses: mad9000/actions-find-and-replace-string@3
         with:
           source: ${{ github.event.head_commit.message }}
           find: 'foo'
           replace: ''
-      - id: sink
+      - id: flow 
         run: |
-          echo "${{steps.source.outputs.value}}"
+          echo "${{steps.summary.outputs.value}}"
+      - id: no-flow
+        run: |
+          echo "${{steps.summary.outputs.foo}}"

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

@@ -33,4 +33,10 @@ jobs:
             echo "$file was changed"
           done
 
+      - name: List all changed files
+        id: no-flow
+        run: |
+          for file in ${{ steps.source.outputs.all_changed_files_count }}; do
+            echo "$file was changed"
+          done
 

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

@@ -22,7 +22,9 @@ jobs:
         run: |
           Write-Output "::set-output name=MSG::$ENV{BODY}"
       - id: step2
-        run: echo "test=${{steps.step1.outputs.MSG}}" >> "$GITHUB_OUTPUT"
+        env:
+            MSG: ${{steps.step1.outputs.MSG}}
+        run: echo "test=$MSG" >> "$GITHUB_OUTPUT"
 
   job2:
     runs-on: ubuntu-latest
@@ -32,5 +34,4 @@ jobs:
     needs: job1
 
     steps:
-      - env:
-        run: echo ${{needs.job1.outputs.job_output}}
+      - run: echo ${{needs.job1.outputs.job_output}}