Extend CFG to reach env expressions

Author: Alvaro Muñoz

ql/lib/codeql/actions/Ast.qll

@@ -184,26 +184,6 @@ 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)
-    )
-  }
 }
 
 /**
@@ -238,7 +218,25 @@ class StepUsesExpr extends StepStmt, UsesExpr {
     )
   }
 
-  override Expression getEnvExpr(string name) { result = this.(StepStmt).getEnvExpr(name) }
+  /**
+   * Gets a environment variable expression by name in the scope of the current step.
+   */
+  override 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)
+    )
+  }
 }
 
 /**
@@ -317,6 +315,26 @@ class RunExpr extends StepStmt, Expression {
   Expression getScriptExpr() { result = scriptExpr }
 
   string getScript() { result = scriptExpr.getValue() }
+
+  /**
+   * Gets a environment variable expression by name in the scope of the current node.
+   */
+  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)
+    )
+  }
 }
 
 /**
@@ -420,5 +438,11 @@ class EnvAccessExpr extends ExprAccessExpr {
 
   EnvAccessExpr() { varName = this.getExpression().regexpCapture("env\\.([A-Za-z0-9_-]+)", 1) }
 
-  override Expression getRefExpr() { exists(RunExpr s | s.getEnvExpr(varName) = result) }
+  override Expression getRefExpr() {
+    exists(JobUsesExpr s | s.getEnvExpr(varName) = result)
+    or
+    exists(StepUsesExpr s | s.getEnvExpr(varName) = result)
+    or
+    exists(RunExpr s | s.getEnvExpr(varName) = result)
+  }
 }

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

@@ -227,7 +227,8 @@ private class StepUsesTree extends StandardPreOrderTree instanceof StepUsesExpr
   override ControlFlowTree getChildNode(int i) {
     result =
       rank[i](Expression child, Location l |
-        child = super.getArgumentExpr(_) and l = child.getLocation()
+        (child = super.getArgumentExpr(_) or child = super.getEnvExpr(_)) and
+        l = child.getLocation()
       |
         child
         order by
@@ -240,7 +241,8 @@ private class JobUsesTree extends StandardPreOrderTree instanceof JobUsesExpr {
   override ControlFlowTree getChildNode(int i) {
     result =
       rank[i](Expression child, Location l |
-        child = super.getArgumentExpr(_) and l = child.getLocation()
+        (child = super.getArgumentExpr(_) or child = super.getEnvExpr(_)) and
+        l = child.getLocation()
       |
         child
         order by

ql/lib/test/test.ql

@@ -31,10 +31,6 @@ query predicate runStepChildren(RunExpr run, AstNode child) { child.getParentNod
 
 query predicate varAccesses(ExprAccessExpr ea, string expr) { expr = ea.getExpression() }
 
-query predicate outputAccesses(StepOutputAccessExpr va, string id, string var) {
-  id = va.getStepId() and var = va.getVarName()
-}
-
 query predicate orphanVarAccesses(ExprAccessExpr va, string var) {
   var = va.getExpression() and
   not exists(AstNode n | n = va.getParentNode())
@@ -53,25 +49,21 @@ query predicate cfgNodes(Cfg::Node n) {
 }
 
 query predicate dfNodes(DataFlow::Node e) {
-  e.getLocation().getFile().getBaseName() = "simple1.yml"
+  e.getLocation().getFile().getBaseName() = "argus_case_study.yml"
 }
 
 query predicate exprNodes(DataFlow::ExprNode e) { any() }
 
 query predicate argumentNodes(DataFlow::ArgumentNode e) { any() }
 
-query predicate localFlow(StepUsesExpr s, StepOutputAccessExpr o) { s.getId() = o.getStepId() }
-
 query predicate usesIds(StepUsesExpr s, string a) { s.getId() = a }
 
-query predicate varIds(StepOutputAccessExpr s, string a) { s.getStepId() = a }
-
 query predicate nodeLocations(DataFlow::Node n, Location l) { n.getLocation() = l }
 
 query predicate scopes(Cfg::CfgScope c) { any() }
 
-query predicate sources(string action, string version, string output, string kind) {
-  sourceModel(action, version, output, kind)
+query predicate sources(string action, string version, string output, string trigger, string kind) {
+  sourceModel(action, version, output, trigger, kind)
 }
 
 query predicate summaries(string action, string version, string input, string output, string kind) {