Merge pull request #17 from GitHubSecurityLab/reusable_workflow_models

feat(reusable-workflow-models): Reusable workflow MaD

Author: Alvaro Muñoz

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

@@ -243,21 +243,9 @@ private class JobTree extends StandardPreOrderTree instanceof JobStmt {
   }
 }
 
-private class StepUsesTree extends StandardPreOrderTree instanceof StepUsesExpr {
-  override ControlFlowTree getChildNode(int i) {
-    result =
-      rank[i](Expression child, Location l |
-        (child = super.getArgumentExpr(_) or child = super.getEnvExpr(_)) and
-        l = child.getLocation()
-      |
-        child
-        order by
-          l.getStartLine(), l.getStartColumn(), l.getEndColumn(), l.getEndLine(), child.toString()
-      )
-  }
-}
+private class UsesExprTree extends LeafTree instanceof UsesExpr { }
 
-private class JobUsesTree extends StandardPreOrderTree instanceof JobUsesExpr {
+private class UsesTree extends StandardPreOrderTree instanceof UsesExpr {
   override ControlFlowTree getChildNode(int i) {
     result =
       rank[i](Expression child, Location l |

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

@@ -58,7 +58,7 @@ class DataFlowExpr extends Cfg::Node {
 }
 
 /**
- * A call corresponds to a Uses steps where a 3rd party action or a reusable workflow gets called
+ * A call corresponds to a Uses steps where a 3rd party action or a reusable workflow get called
  */
 class DataFlowCall instanceof Cfg::Node {
   DataFlowCall() { super.getAstNode() instanceof UsesExpr }
@@ -180,6 +180,23 @@ predicate stepsCtxLocalStep(Node nodeFrom, Node nodeTo) {
   )
 }
 
+/**
+ * Holds if there is a local flow step between a ${{ needs.xxx.outputs.yyy }} expression accesing a job output field
+ * and the step output itself. But only for those cases where the job (needs) output is defined externally in a MaD Source
+ * 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 we cannot write a Source that stores the taint in a Content, we can only do that for steps
+ * (storeStep). The easiest thing is to add this local flow step that simulates a read step from the source node for a specific
+ * field name.
+ */
+predicate needsCtxLocalStep(Node nodeFrom, Node nodeTo) {
+  exists(UsesExpr astFrom, NeedsCtxAccessExpr astTo |
+    externallyDefinedSource(nodeFrom, _, "output." + astTo.getFieldName()) and
+    astFrom = nodeFrom.asExpr() and
+    astTo = nodeTo.asExpr() and
+    astTo.getRefExpr() = astFrom
+  )
+}
+
 /**
  * Holds if there is a local flow step between a ${{}} expression accesing an input variable and the input itself
  * e.g. ${{ inputs.foo }}
@@ -215,6 +232,7 @@ predicate envCtxLocalStep(Node nodeFrom, Node nodeTo) {
 pragma[nomagic]
 predicate localFlowStep(Node nodeFrom, Node nodeTo) {
   stepsCtxLocalStep(nodeFrom, nodeTo) or
+  needsCtxLocalStep(nodeFrom, nodeTo) or
   inputsCtxLocalStep(nodeFrom, nodeTo) or
   envCtxLocalStep(nodeFrom, nodeTo)
 }

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

@@ -66,6 +66,17 @@ class ParameterNode extends ExprNode {
   InputExpr getInputExpr() { result = input }
 }
 
+/**
+ * A call to a data flow callable (Uses).
+ */
+class CallNode extends ExprNode {
+  private DataFlowCall call;
+
+  CallNode() { this.getCfgNode() instanceof DataFlowCall }
+
+  string getCallee() { result = this.getCfgNode().(DataFlowCall).getName() }
+}
+
 /**
  * An argument to a Uses step (call).
  */

ql/lib/ext/TEST-RW-MODELS.model.yml

@@ -0,0 +1,17 @@
+extensions:
+  - addsTo:
+      pack: githubsecuritylab/actions-all
+      extensible: summaryModel
+    data:
+      - ["octo-org/this-repo/.github/workflows/workflow.yml", "*", "input.config-path", "output.workflow-output", "taint"]
+      - ["octo-org/summary-repo/.github/workflows/workflow.yml", "*", "input.config-path", "output.workflow-output", "taint"]
+  - addsTo:
+      pack: githubsecuritylab/actions-all
+      extensible: sourceModel
+    data:
+      - ["octo-org/source-repo/.github/workflows/workflow.yml", "*", "output.workflow-output", "*", "Foo"]
+  - addsTo:
+      pack: githubsecuritylab/actions-all
+      extensible: sinkModel
+    data:
+      - ["octo-org/sink-repo/.github/workflows/workflow.yml", "*", "input.config-path", "expression-injection"]

ql/lib/test/test.ql

@@ -43,14 +43,9 @@ query predicate nonOrphanVarAccesses(ExprAccessExpr va, string var, AstNode pare
 
 query predicate parentNodes(AstNode child, AstNode parent) { child.getParentNode() = parent }
 
-query predicate cfgNodes(Cfg::Node n) {
-  //any()
-  n.getAstNode() instanceof OutputsStmt
-}
+query predicate cfgNodes(Cfg::Node n) { any() }
 
-query predicate dfNodes(DataFlow::Node e) {
-  e.getLocation().getFile().getBaseName() = "argus_case_study.yml"
-}
+query predicate dfNodes(DataFlow::Node e) { any() }
 
 query predicate exprNodes(DataFlow::ExprNode e) { any() }
 
@@ -69,3 +64,7 @@ query predicate sources(string action, string version, string output, string tri
 query predicate summaries(string action, string version, string input, string output, string kind) {
   summaryModel(action, version, input, output, kind)
 }
+
+query predicate calls(DataFlow::CallNode call, string callee) { callee = call.getCallee() }
+
+query predicate needs(DataFlow::ExprNode e) { e.asExpr() instanceof NeedsCtxAccessExpr }

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

@@ -8,17 +8,20 @@ jobs:
     uses: octo-org/this-repo/.github/workflows/reusable_workflow.yml@172239021f7ba04fe7327647b213799853a9eb89
     with:
       config-path: ${{ github.event.pull_request.head.ref }}
-    secrets: inherit
   call2:
     uses: ./.github/workflows/reusable_workflow.yml
     with:
       config-path: ${{ github.event.pull_request.head.ref }}
-    secrets: inherit
   call3:
-    uses: octo-org/another-repo/.github/workflows/workflow.yml@v1
+    uses: octo-org/summary-repo/.github/workflows/workflow.yml@v1
+    with:
+      config-path: ${{ github.event.pull_request.head.ref }}
+  call4:
+    uses: octo-org/source-repo/.github/workflows/workflow.yml@v1
+  call5:
+    uses: octo-org/sink-repo/.github/workflows/workflow.yml@v1
     with:
       config-path: ${{ github.event.pull_request.head.ref }}
-    secrets: inherit
 
   job1:
     runs-on: ubuntu-latest
@@ -36,3 +39,8 @@ jobs:
     needs: call3
     steps:
       - run: echo ${{ needs.call3.outputs.workflow-output }}
+  job4:
+    runs-on: ubuntu-latest
+    needs: call4
+    steps:
+      - run: echo ${{ needs.call4.outputs.workflow-output }}