Refactor MaD semantics

ql/lib/codeql/actions/Ast.qll

@@ -115,6 +115,9 @@ 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) }
 
@@ -181,6 +184,26 @@ class StepStmt extends Statement instanceof Actions::Step {
   string getId() { result = super.getId() }
 
   JobStmt getJobStmt() { result = super.getJob() }
+
+  /**
+   * Gets a environment variable expression by name in the scope of the current step.
+   */
+  Expression getEnvExpr(string name) {
+    exists(Actions::StepEnv env |
+      env.getStep() = this and
+      env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
+    )
+    or
+    exists(Actions::JobEnv env |
+      env.getJob() = this.getJobStmt() and
+      env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
+    )
+    or
+    exists(Actions::WorkflowEnv env |
+      env.getWorkflow() = this.getJobStmt().getWorkflowStmt() and
+      env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
+    )
+  }
 }
 
 /**
@@ -192,6 +215,8 @@ abstract class UsesExpr extends Expression {
   abstract string getVersion();
 
   abstract Expression getArgumentExpr(string key);
+
+  abstract Expression getEnvExpr(string name);
 }
 
 /**
@@ -212,6 +237,8 @@ class StepUsesExpr extends StepStmt, UsesExpr {
       result = with.lookup(key)
     )
   }
+
+  override Expression getEnvExpr(string name) { result = this.(StepStmt).getEnvExpr(name) }
 }
 
 /**
@@ -260,6 +287,23 @@ class JobUsesExpr extends UsesExpr instanceof YamlMapping {
   override Expression getArgumentExpr(string key) {
     this.(YamlMapping).lookup("with").(YamlMapping).lookup(key) = result
   }
+
+  /**
+   * Gets a environment variable expression by name in the scope of the current node.
+   */
+  override Expression getEnvExpr(string name) {
+    this.(YamlMapping).lookup("env").(YamlMapping).lookup(name) = result
+    or
+    exists(Actions::JobEnv env |
+      env.getJob() = this.getJobStmt() and
+      env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
+    )
+    or
+    exists(Actions::WorkflowEnv env |
+      env.getWorkflow() = this.getJobStmt().getWorkflowStmt() and
+      env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
+    )
+  }
 }
 
 /**
@@ -272,13 +316,6 @@ class RunExpr extends StepStmt, Expression {
 
   Expression getScriptExpr() { result = scriptExpr }
 
-  Expression getEnvExpr(string name) {
-    exists(Actions::StepEnv env |
-      env.getStep() = this and
-      env.(YamlMapping).maps(any(YamlScalar s | s.getValue() = name), result)
-    )
-  }
-
   string getScript() { result = scriptExpr.getValue() }
 }
 

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

@@ -3,8 +3,8 @@ import codeql.actions.DataFlow
 import actions
 
 /** Holds if a source model exists for the given parameters. */
-predicate sourceModel(string action, string version, string output, string kind) {
-  Extensions::sourceModel(action, version, output, kind)
+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. */
@@ -17,15 +17,27 @@ predicate sinkModel(string action, string version, string input, string kind) {
   Extensions::sinkModel(action, version, input, kind)
 }
 
+/**
+ * MaD sinks
+ * Fields:
+ *    - action: Fully-qualified action name (NWO)
+ *    - version: Either '*' or a specific SHA/Tag
+ *    - input arg: sink node (prefixed with either `env.` or `input.`)
+ *    - kind: sink kind
+ */
 predicate sinkNode(DataFlow::ExprNode sink, string kind) {
   exists(UsesExpr uses, string action, string version, string input |
-    uses.getArgumentExpr(input.splitAt(",").trim()) = sink.asExpr() and
+    (
+      if input.trim().matches("env.%")
+      then sink.asExpr() = uses.getEnvExpr(input.trim().replaceAll("input\\.", ""))
+      else sink.asExpr() = uses.getArgumentExpr(input.trim())
+    ) and
     sinkModel(action, version, input, kind) and
     uses.getCallee() = action and
     (
       if version.trim() = "*"
       then uses.getVersion() = any(string v)
-      else uses.getVersion() = version.splitAt(",").trim()
+      else uses.getVersion() = version.trim()
     )
   )
 }

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

@@ -125,19 +125,36 @@ private class EventSource extends RemoteFlowSource {
   override string getSourceType() { result = "User-controlled events" }
 }
 
+/**
+ * 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.
+ */
 private class ExternallyDefinedSource extends RemoteFlowSource {
   string soutceType;
 
   ExternallyDefinedSource() {
-    exists(UsesExpr uses, string action, string version, /*string output,*/ string kind |
-      sourceModel(action, version, _, kind) and
+    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.splitAt(",").trim()
+        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
-      uses = this.asExpr() and
       soutceType = kind
     )
   }

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

@@ -21,16 +21,32 @@ 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 |
-    /*, string output */ summaryModel(action, version, input, _, "taint") and
+    // `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.splitAt(",").trim()
+      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
-    pred.asExpr() = uses.getArgumentExpr(input.splitAt(",").trim()) and
     succ.asExpr() = uses
   )
 }

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

@@ -5,7 +5,9 @@
 /**
  * Holds if a source model exists for the given parameters.
  */
-extensible predicate sourceModel(string action, string version, string output, string kind);
+extensible predicate sourceModel(
+  string action, string version, string output, string trigger, string kind
+);
 
 /**
  * Holds if a summary model exists for the given parameters.

ql/lib/ext/REMOVEME.model.yml

@@ -0,0 +1,6 @@
+extensions:
+  - addsTo:
+      pack: codeql/actions-all
+      extensible: sinkModel
+    data:
+      - [ "FAKE-mad9000/actions-find-and-replace-string", "*", "source", "expression-injection" ]

ql/lib/ext/frabert-replace-string-action.model.yml

@@ -0,0 +1,7 @@
+extensions:
+  - addsTo:
+      pack: codeql/actions-all
+      extensible: summaryModel
+    data:
+      - [ "frabert/replace-string-action", "*", "string", "replaced", "taint" ]
+      - [ "frabert/replace-string-action", "*", "replace-with", "replaced", "taint" ]

ql/lib/ext/mad9000-actions-find-and-replace-string.model.yml

@@ -0,0 +1,9 @@
+extensions:
+  - addsTo:
+      pack: codeql/actions-all
+      extensible: summaryModel
+    data:
+      - [ "mad9000/actions-find-and-replace-string", "*", "source", "value", "taint" ]
+      - [ "mad9000/actions-find-and-replace-string", "*", "replace", "value", "taint" ]
+      - [ "frabert/replace-string-action", "*", "string", "replaced", "taint" ]
+      - [ "frabert/replace-string-action", "*", "replace-with", "replaced", "taint" ]

ql/lib/ext/sinks.model.yml

@@ -1,11 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/actions-all
-      extensible: sinkModel
-    data:
-      - [
-          "FAKE-mad9000/actions-find-and-replace-string",
-          "*",
-          "source",
-          "expression-injection",
-        ]

ql/lib/ext/sources.model.yml

@@ -1,11 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/actions-all
-      extensible: sourceModel
-    data:
-      - [
-          "tj-actions/changed-files",
-          "v10, v20, v30, v40",
-          "all_changed_file",
-          "PR",
-        ]

ql/lib/ext/summaries.model.yml

@@ -1,19 +0,0 @@
-extensions:
-  - addsTo:
-      pack: codeql/actions-all
-      extensible: summaryModel
-    data:
-      - [
-          "mad9000/actions-find-and-replace-string",
-          "*",
-          "source, replace",
-          "value",
-          "taint",
-        ]
-      - [
-          "frabert/replace-string-action",
-          "*",
-          "string, replace-with",
-          "replaced",
-          "taint",
-        ]

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

@@ -0,0 +1,28 @@
+extensions:
+  - addsTo:
+      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" ]