Merge pull request #24 from GitHubSecurityLab/matrix_ctx

matrix ctx

Author: Alvaro Muñoz

build-test-dbs.sh

@@ -1,4 +1,6 @@
 #!/bin/bash
+rm -rf ql/lib/test/test.testproj || true
+rm -rf ql/src/test/test.testproj || true
 rm -rf src-test.testproj || true
 rm -rf lib-test.testproj || true
 codeql database create src-test.testproj -l yaml -s ql/src/test

ql/lib/codeql/actions/Ast.qll

@@ -81,6 +81,8 @@ class WorkflowStmt extends Statement instanceof Actions::Workflow {
   }
 
   Statement getPermissionsStmt() { result = this.(YamlMapping).lookup("permissions") }
+
+  StrategyStmt getStrategyStmt() { result = this.(YamlMapping).lookup("strategy") }
 }
 
 class ReusableWorkflowStmt extends WorkflowStmt {
@@ -125,6 +127,23 @@ class OutputsStmt extends Statement instanceof YamlMapping {
   string getAnOutputName() { this.(YamlMapping).maps(any(YamlString s | s.getValue() = result), _) }
 }
 
+class StrategyStmt extends Statement instanceof YamlMapping {
+  YamlMapping parent;
+
+  StrategyStmt() { parent.lookup("strategy") = this }
+
+  /**
+   * Gets a specific matric expression (YamlMapping) by name.
+   */
+  MatrixVariableExpr getMatrixVariableExpr(string name) {
+    this.(YamlMapping).lookup("matrix").(YamlMapping).lookup(name) = result
+  }
+
+  string getAMatrixVariableName() {
+    this.(YamlMapping).maps(any(YamlString s | s.getValue() = result), _)
+  }
+}
+
 class InputExpr extends Expression instanceof YamlString {
   InputExpr() { exists(InputsStmt inputs | inputs.(YamlMapping).maps(this, _)) }
 }
@@ -138,6 +157,14 @@ class OutputExpr extends Expression instanceof YamlString {
   }
 }
 
+class MatrixVariableExpr extends Expression instanceof YamlString {
+  MatrixVariableExpr() {
+    exists(StrategyStmt outputs |
+      outputs.(YamlMapping).lookup("matrix").(YamlMapping).lookup(_) = this
+    )
+  }
+}
+
 /**
  * A Job is a collection of steps that run in an execution environment.
  */
@@ -191,6 +218,8 @@ class JobStmt extends Statement instanceof Actions::Job {
   IfStmt getIfStmt() { result = super.getIf() }
 
   Statement getPermissionsStmt() { result = this.(YamlMapping).lookup("permissions") }
+
+  StrategyStmt getStrategyStmt() { result = this.(YamlMapping).lookup("strategy") }
 }
 
 /**
@@ -332,7 +361,8 @@ class ExprAccessExpr extends Expression instanceof YamlString {
 class CtxAccessExpr extends ExprAccessExpr {
   CtxAccessExpr() {
     expr.regexpMatch([
-        stepsCtxRegex(), needsCtxRegex(), jobsCtxRegex(), envCtxRegex(), inputsCtxRegex()
+        stepsCtxRegex(), needsCtxRegex(), jobsCtxRegex(), envCtxRegex(), inputsCtxRegex(),
+        matrixCtxRegex()
       ])
   }
 
@@ -342,22 +372,28 @@ class CtxAccessExpr extends ExprAccessExpr {
 }
 
 private string stepsCtxRegex() {
-  result = "\\bsteps\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)\\b"
+  result = wrapRegexp("steps\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)")
 }
 
 private string needsCtxRegex() {
-  result = "\\bneeds\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)\\b"
+  result = wrapRegexp("needs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)")
 }
 
 private string jobsCtxRegex() {
-  result = "\\bjobs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)\\b"
+  result = wrapRegexp("jobs\\.([A-Za-z0-9_-]+)\\.outputs\\.([A-Za-z0-9_-]+)")
 }
 
-private string envCtxRegex() { result = "\\benv\\.([A-Za-z0-9_-]+)\\b" }
+private string envCtxRegex() { result = wrapRegexp("env\\.([A-Za-z0-9_-]+)") }
+
+private string matrixCtxRegex() { result = wrapRegexp("matrix\\.([A-Za-z0-9_-]+)") }
 
 private string inputsCtxRegex() {
-  result = "\\binputs\\.([A-Za-z0-9_-]+)\\b" or
-  result = "\\bgithub\\.event\\.inputs\\.([A-Za-z0-9_-]+)\\b"
+  result = wrapRegexp(["inputs\\.([A-Za-z0-9_-]+)", "github\\.event\\.inputs\\.([A-Za-z0-9_-]+)"])
+}
+
+bindingset[regex]
+private string wrapRegexp(string regex) {
+  result = ["\\b" + regex + "\\b", "fromJSON\\(" + regex + "\\)", "toJSON\\(" + regex + "\\)"]
 }
 
 /**
@@ -487,3 +523,31 @@ class EnvCtxAccessExpr extends CtxAccessExpr {
     )
   }
 }
+
+/**
+ * Holds for an expression accesing the `matrix` context.
+ * https://docs.github.com/en/actions/learn-github-actions/contexts#context-availability
+ * e.g. `${{ matrix.foo }}`
+ */
+class MatrixCtxAccessExpr extends CtxAccessExpr {
+  string fieldName;
+
+  MatrixCtxAccessExpr() {
+    expr.regexpMatch(matrixCtxRegex()) and
+    fieldName = expr.regexpCapture(matrixCtxRegex(), 1)
+  }
+
+  override string getFieldName() { result = fieldName }
+
+  override Expression getRefExpr() {
+    exists(WorkflowStmt w |
+      w.getStrategyStmt().getMatrixVariableExpr(fieldName) = result and
+      w.getAChildNode*() = this
+    )
+    or
+    exists(JobStmt j |
+      j.getStrategyStmt().getMatrixVariableExpr(fieldName) = result and
+      j.getAChildNode*() = this
+    )
+  }
+}

ql/lib/codeql/actions/controlflow/internal/Cfg.qll

@@ -174,6 +174,7 @@ private class WorkflowTree extends StandardPreOrderTree instanceof WorkflowStmt
           (
             child = this.(ReusableWorkflowStmt).getInputsStmt() or
             child = this.(ReusableWorkflowStmt).getOutputsStmt() or
+            child = this.(ReusableWorkflowStmt).getStrategyStmt() or
             child = this.(ReusableWorkflowStmt).getAJobStmt()
           ) and
           l = child.getLocation()
@@ -185,7 +186,10 @@ private class WorkflowTree extends StandardPreOrderTree instanceof WorkflowStmt
     else
       result =
         rank[i](Expression child, Location l |
-          child = super.getAJobStmt() and
+          (
+            child = super.getAJobStmt() or
+            child = super.getStrategyStmt()
+          ) and
           l = child.getLocation()
         |
           child
@@ -225,13 +229,29 @@ private class OutputsTree extends StandardPreOrderTree instanceof OutputsStmt {
 
 private class OutputExprTree extends LeafTree instanceof OutputExpr { }
 
+private class StrategyTree extends StandardPreOrderTree instanceof StrategyStmt {
+  override ControlFlowTree getChildNode(int i) {
+    result =
+      rank[i](Expression child, Location l |
+        child = super.getMatrixVariableExpr(_) and l = child.getLocation()
+      |
+        child
+        order by
+          l.getStartLine(), l.getStartColumn(), l.getEndColumn(), l.getEndLine(), child.toString()
+      )
+  }
+}
+
+private class MatrixVariableExprTree extends LeafTree instanceof MatrixVariableExpr { }
+
 private class JobTree extends StandardPreOrderTree instanceof JobStmt {
   override ControlFlowTree getChildNode(int i) {
     result =
       rank[i](Expression child, Location l |
         (
           child = super.getAStepStmt() or
           child = super.getOutputsStmt() or
+          child = super.getStrategyStmt() or
           child = super.getUsesExpr()
         ) and
         l = child.getLocation()

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

@@ -133,7 +133,7 @@ predicate typeStrongerThan(DataFlowType t1, DataFlowType t2) { none() }
 
 newtype TContent =
   TFieldContent(string name) {
-    // We only use field flow for steps and jobs outputs, not for accessing other context fields such as env or inputs
+    // We only use field flow for steps and jobs outputs, not for accessing other context fields such as env, matrix or inputs
     name = any(StepsCtxAccessExpr a).getFieldName() or
     name = any(NeedsCtxAccessExpr a).getFieldName() or
     name = any(JobsCtxAccessExpr a).getFieldName()
@@ -209,6 +209,18 @@ predicate inputsCtxLocalStep(Node nodeFrom, Node nodeTo) {
   )
 }
 
+/**
+ * Holds if there is a local flow step between a ${{}} expression accesing a matrix variable and the matrix itself
+ * e.g. ${{ matrix.foo }}
+ */
+predicate matrixCtxLocalStep(Node nodeFrom, Node nodeTo) {
+  exists(Expression astFrom, MatrixCtxAccessExpr astTo |
+    astFrom = nodeFrom.asExpr() and
+    astTo = nodeTo.asExpr() and
+    astTo.getRefExpr() = astFrom
+  )
+}
+
 /**
  * Holds if there is a local flow step between a ${{}} expression accesing an env var and the var definition itself
  * e.g. ${{ env.foo }}
@@ -234,6 +246,7 @@ predicate localFlowStep(Node nodeFrom, Node nodeTo) {
   stepsCtxLocalStep(nodeFrom, nodeTo) or
   needsCtxLocalStep(nodeFrom, nodeTo) or
   inputsCtxLocalStep(nodeFrom, nodeTo) or
+  matrixCtxLocalStep(nodeFrom, nodeTo) or
   envCtxLocalStep(nodeFrom, nodeTo)
 }
 

ql/src/test/partial.ql renamed to ql/src/Debug/partial.ql

@@ -15,7 +15,7 @@ import PartialFlow::PartialPathGraph
 private module MyConfig implements DataFlow::ConfigSig {
   predicate isSource(DataFlow::Node source) {
     source instanceof RemoteFlowSource and
-    source.getLocation().getFile().getBaseName() = "calling_workflow.yml"
+    source.getLocation().getFile().getBaseName() = "matrix.yml"
   }
 
   predicate isSink(DataFlow::Node sink) { none() }

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

@@ -0,0 +1,42 @@
+name: "CodeQL Auto Language"
+
+on:
+  push:
+    branches: [ main ]
+  pull_request:
+    branches: [ main ]
+  schedule:
+    - cron: '17 19 * * 6'
+
+jobs:
+  create-matrix:
+    runs-on: ubuntu-latest
+    outputs:
+      matrix: ${{ steps.set-matrix.outputs.all_changed_files }}
+    steps:
+      - name: Get changed files
+        id: set-matrix
+        uses: tj-actions/changed-files@v40
+          
+  analyze:
+    needs: create-matrix
+    if: ${{ needs.create-matrix.outputs.matrix != '[]' }}
+    name: Analyze
+    runs-on: ubuntu-latest
+    permissions:
+      actions: read
+      contents: read
+      security-events: write
+
+    strategy:
+      fail-fast: false
+      matrix: 
+        language: ${{ fromJSON(needs.create-matrix.outputs.matrix) }}
+
+    steps:
+    - name: Checkout repository
+      uses: actions/checkout@v3
+
+    # Initializes the CodeQL tools for scanning.
+    - run: | 
+        ${{ matrix.language }}