Merge branch 'master' into immutable-actions

Author: KyFaSt

ql/lib/codeql/actions/Bash.qll

@@ -256,20 +256,20 @@ class BashShellScript extends ShellScript {
 
   override predicate getAWriteToGitHubEnv(string name, string data) {
     exists(string raw |
-      Bash::extractFileWrite(this.getRawScript(), "GITHUB_ENV", raw) and
+      Bash::extractFileWrite(this, "GITHUB_ENV", raw) and
       Bash::extractVariableAndValue(raw, name, data)
     )
   }
 
   override predicate getAWriteToGitHubOutput(string name, string data) {
     exists(string raw |
-      Bash::extractFileWrite(this.getRawScript(), "GITHUB_OUTPUT", raw) and
+      Bash::extractFileWrite(this, "GITHUB_OUTPUT", raw) and
       Bash::extractVariableAndValue(raw, name, data)
     )
   }
 
   override predicate getAWriteToGitHubPath(string data) {
-    Bash::extractFileWrite(this.getRawScript(), "GITHUB_PATH", data)
+    Bash::extractFileWrite(this, "GITHUB_PATH", data)
   }
 
   override predicate getAnEnvReachingGitHubOutputWrite(string var, string output_field) {
@@ -542,12 +542,12 @@ module Bash {
     blockFileWrite(script, cmd, file, content, filters)
   }
 
-  bindingset[script, file_var]
-  predicate extractFileWrite(string script, string file_var, string content) {
+  bindingset[file_var]
+  predicate extractFileWrite(BashShellScript script, string file_var, string content) {
     // single line assignment
     exists(string file_expr, string raw_content |
       isParameterExpansion(file_expr, file_var, _, _) and
-      singleLineFileWrite(script.splitAt("\n"), _, file_expr, raw_content, _) and
+      singleLineFileWrite(script.getAStmt(), _, file_expr, raw_content, _) and
       content = trimQuotes(raw_content)
     )
     or
@@ -566,12 +566,12 @@ module Bash {
         cmd = "add-path" and
         content = value
       ) and
-      singleLineWorkflowCmd(script.splitAt("\n"), cmd, key, value)
+      singleLineWorkflowCmd(script.getAStmt(), cmd, key, value)
     )
     or
     // multiline assignment
     exists(string file_expr, string raw_content |
-      multiLineFileWrite(script, _, file_expr, raw_content, _) and
+      multiLineFileWrite(script.getRawScript(), _, file_expr, raw_content, _) and
       isParameterExpansion(file_expr, file_var, _, _) and
       content = trimQuotes(raw_content)
     )
@@ -691,11 +691,32 @@ module Bash {
       // echo "FIELD=${VAR2:-default}" >> $GITHUB_ENV (field, file_write_value)
       script.getAnAssignment(var2, value2) and
       containsCmdSubstitution(value2, cmd) and
-      containsParameterExpansion(expr, var2, _, _)
+      containsParameterExpansion(expr, var2, _, _) and
+      not varMatchesRegexTest(script, var2, alphaNumericRegex())
     )
     or
     // var reaches the file write directly
     // echo "FIELD=$(cmd)" >> $GITHUB_ENV (field, file_write_value)
     containsCmdSubstitution(expr, cmd)
   }
+
+  /**
+   * Holds if there test command that checks a variable against a regex
+   * eg: `[[ $VAR =~ ^[a-zA-Z0-9_]+$ ]]`
+   */
+  bindingset[var, regex]
+  predicate varMatchesRegexTest(BashShellScript script, string var, string regex) {
+    exists(string lhs, string rhs |
+      lhs = script.getACommand().regexpCapture(".*\\[\\[\\s*(.*?)\\s*=~\\s*(.*?)\\s*\\]\\].*", 1) and
+      containsParameterExpansion(lhs, var, _, _) and
+      rhs = script.getACommand().regexpCapture(".*\\[\\[\\s*(.*?)\\s*=~\\s*(.*?)\\s*\\]\\].*", 2) and
+      trimQuotes(rhs).regexpMatch(regex)
+    )
+  }
+
+  /**
+   * Holds if the given regex is used to match an alphanumeric string
+   * eg: `^[0-9a-zA-Z]{40}$`, `^[0-9]+$` or `^[a-zA-Z0-9_]+$`
+   */
+  string alphaNumericRegex() { result = "^\\^\\[([09azAZ_-]+)\\](\\+|\\{\\d+\\})\\$$" }
 }

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

@@ -4,20 +4,21 @@ private import codeql.actions.Helper
 private import codeql.actions.config.Config
 private import codeql.actions.DataFlow
 
+bindingset[text]
+int numberOfLines(string text) { result = max(int i | exists(text.splitAt("\n", i))) }
+
 /**
  * Gets the length of each line in the StringValue .
  */
 bindingset[text]
-int lineLength(string text, int idx) {
-  exists(string line | line = text.splitAt("\n", idx) and result = line.length() + 1)
-}
+int lineLength(string text, int i) { result = text.splitAt("\n", i).length() + 1 }
 
 /**
  * Gets the sum of the length of the lines up to the given index.
  */
 bindingset[text]
 int partialLineLengthSum(string text, int i) {
-  i in [0 .. count(text.splitAt("\n"))] and
+  i in [0 .. numberOfLines(text)] and
   result = sum(int j, int length | j in [0 .. i] and length = lineLength(text, j) | length)
 }
 
@@ -114,7 +115,11 @@ abstract class AstNodeImpl extends TAstNode {
   /**
    * Gets and Event triggering this node.
    */
-  EventImpl getATriggerEvent() { result = this.getEnclosingJob().getATriggerEvent() }
+  EventImpl getATriggerEvent() {
+    result = this.getEnclosingJob().getATriggerEvent()
+    or
+    not exists(this.getEnclosingJob()) and result = this.getEnclosingWorkflow().getATriggerEvent()
+  }
 
   /**
    * Gets the enclosing Step.
@@ -1631,7 +1636,7 @@ class StepsExpressionImpl extends SimpleReferenceExpressionImpl {
     exists(string expr |
       (
         exists(getAJsonReferenceExpression(expression, _)) and
-        expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1)
+        expr = normalizeExpr(expression).regexpCapture("(?i)(from|to)json\\((.*)\\).*", 2)
         or
         exists(getASimpleReferenceExpression(expression, _)) and
         expr = normalizeExpr(expression)
@@ -1672,7 +1677,7 @@ class NeedsExpressionImpl extends SimpleReferenceExpressionImpl {
     exists(string expr |
       (
         exists(getAJsonReferenceExpression(expression, _)) and
-        expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1)
+        expr = normalizeExpr(expression).regexpCapture("(?i)(from|to)json\\((.*)\\).*", 2)
         or
         exists(getASimpleReferenceExpression(expression, _)) and
         expr = normalizeExpr(expression)
@@ -1716,7 +1721,7 @@ class JobsExpressionImpl extends SimpleReferenceExpressionImpl {
     exists(string expr |
       (
         exists(getAJsonReferenceExpression(expression, _)) and
-        expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1)
+        expr = normalizeExpr(expression).regexpCapture("(?i)(from|to)json\\((.*)\\).*", 2)
         or
         exists(getASimpleReferenceExpression(expression, _)) and
         expr = normalizeExpr(expression)
@@ -1775,7 +1780,7 @@ class EnvExpressionImpl extends SimpleReferenceExpressionImpl {
     exists(string expr |
       (
         exists(getAJsonReferenceExpression(expression, _)) and
-        expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1)
+        expr = normalizeExpr(expression).regexpCapture("(?i)(from|to)json\\((.*)\\).*", 2)
         or
         exists(getASimpleReferenceExpression(expression, _)) and
         expr = normalizeExpr(expression)
@@ -1810,7 +1815,7 @@ class MatrixExpressionImpl extends SimpleReferenceExpressionImpl {
     exists(string expr |
       (
         exists(getAJsonReferenceExpression(expression, _)) and
-        expr = normalizeExpr(expression).regexpCapture("(?i)fromjson\\((.*)\\).*", 1)
+        expr = normalizeExpr(expression).regexpCapture("(?i)(from|to)json\\((.*)\\).*", 2)
         or
         exists(getASimpleReferenceExpression(expression, _)) and
         expr = normalizeExpr(expression)

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

@@ -18,39 +18,47 @@ abstract class RemoteFlowSource extends SourceNode {
   /** Gets a string that describes the type of this remote flow source. */
   abstract string getSourceType();
 
+  /** Gets the event that triggered the source. */
+  abstract Event getEvent();
+
   override string getThreatModel() { result = "remote" }
 }
 
 class GitHubCtxSource extends RemoteFlowSource {
   string flag;
+  Event event;
 
   GitHubCtxSource() {
     exists(Expression e, string context, string context_prefix |
       this.asExpr() = e and
       context = e.getExpression() and
+      event = e.getEnclosingWorkflow().getATriggerEvent() and
       normalizeExpr(context) = "github.head_ref" and
-      contextTriggerDataModel(e.getEnclosingWorkflow().getATriggerEvent().getName(), context_prefix) and
+      contextTriggerDataModel(event.getName(), context_prefix) and
       normalizeExpr(context).matches("%" + context_prefix + "%") and
       flag = "branch"
     )
   }
 
   override string getSourceType() { result = flag }
+
+  override Event getEvent() { result = event }
 }
 
 class GitHubEventCtxSource extends RemoteFlowSource {
   string flag;
   string context;
+  Event event;
 
   GitHubEventCtxSource() {
     exists(Expression e, string regexp |
       this.asExpr() = e and
       context = e.getExpression() and
+      event = e.getATriggerEvent() and
       (
         // the context is available for the job trigger events
         exists(string context_prefix |
-          contextTriggerDataModel(e.getEnclosingWorkflow().getATriggerEvent().getName(),
-            context_prefix) and
+          contextTriggerDataModel(event.getName(), context_prefix) and
           normalizeExpr(context).matches("%" + context_prefix + "%")
         )
         or
@@ -65,12 +73,16 @@ class GitHubEventCtxSource extends RemoteFlowSource {
   override string getSourceType() { result = flag }
 
   string getContext() { result = context }
+
+  override Event getEvent() { result = event }
 }
 
 abstract class CommandSource extends RemoteFlowSource {
   abstract string getCommand();
 
   abstract Run getEnclosingRun();
+
+  override Event getEvent() { result = this.getEnclosingRun().getATriggerEvent() }
 }
 
 class GitCommandSource extends RemoteFlowSource, CommandSource {
@@ -181,18 +193,19 @@ class GitHubEventPathSource extends RemoteFlowSource, CommandSource {
 
 class GitHubEventJsonSource extends RemoteFlowSource {
   string flag;
+  Event event;
 
   GitHubEventJsonSource() {
     exists(Expression e, string context, string regexp |
       this.asExpr() = e and
       context = e.getExpression() and
+      event = e.getEnclosingWorkflow().getATriggerEvent() and
       untrustedEventPropertiesDataModel(regexp, _) and
       (
         // only contexts for the triggering events are considered tainted.
         // eg: for `pull_request`, we only consider `github.event.pull_request`
         exists(string context_prefix |
-          contextTriggerDataModel(e.getEnclosingWorkflow().getATriggerEvent().getName(),
-            context_prefix) and
+          contextTriggerDataModel(event.getName(), context_prefix) and
           normalizeExpr(context).matches("%" + context_prefix + "%")
         ) and
         normalizeExpr(context).regexpMatch("(?i).*" + wrapJsonRegexp(regexp) + ".*")
@@ -206,6 +219,8 @@ class GitHubEventJsonSource extends RemoteFlowSource {
   }
 
   override string getSourceType() { result = flag }
+
+  override Event getEvent() { result = event }
 }
 
 /**
@@ -217,6 +232,8 @@ class MaDSource extends RemoteFlowSource {
   MaDSource() { madSource(this, sourceType, _) }
 
   override string getSourceType() { result = sourceType }
+
+  override Event getEvent() { result = this.asExpr().getATriggerEvent() }
 }
 
 abstract class FileSource extends RemoteFlowSource { }
@@ -228,12 +245,16 @@ class ArtifactSource extends RemoteFlowSource, FileSource {
   ArtifactSource() { this.asExpr() instanceof UntrustedArtifactDownloadStep }
 
   override string getSourceType() { result = "artifact" }
+
+  override Event getEvent() { result = this.asExpr().getATriggerEvent() }
 }
 
 /**
  * A file from an untrusted checkout.
  */
 private class CheckoutSource extends RemoteFlowSource, FileSource {
+  Event event;
+
   CheckoutSource() {
     // This should be:
     // source instanceof PRHeadCheckoutStep
@@ -245,7 +266,8 @@ private class CheckoutSource extends RemoteFlowSource, FileSource {
       uses.getCallee() = "actions/checkout" and
       exists(uses.getArgument("ref")) and
       not uses.getArgument("ref").matches("%base%") and
-      uses.getATriggerEvent().getName() = checkoutTriggers()
+      event = uses.getATriggerEvent() and
+      event.getName() = checkoutTriggers()
     )
     or
     this.asExpr() instanceof GitMutableRefCheckout
@@ -258,6 +280,8 @@ private class CheckoutSource extends RemoteFlowSource, FileSource {
   }
 
   override string getSourceType() { result = "artifact" }
+
+  override Event getEvent() { result = event }
 }
 
 /**
@@ -273,6 +297,8 @@ class DornyPathsFilterSource extends RemoteFlowSource {
   }
 
   override string getSourceType() { result = "filename" }
+
+  override Event getEvent() { result = this.asExpr().getATriggerEvent() }
 }
 
 /**
@@ -294,6 +320,8 @@ class TJActionsChangedFilesSource extends RemoteFlowSource {
   }
 
   override string getSourceType() { result = "filename" }
+
+  override Event getEvent() { result = this.asExpr().getATriggerEvent() }
 }
 
 /**
@@ -315,6 +343,8 @@ class TJActionsVerifyChangedFilesSource extends RemoteFlowSource {
   }
 
   override string getSourceType() { result = "filename" }
+
+  override Event getEvent() { result = this.asExpr().getATriggerEvent() }
 }
 
 class Xt0rtedSlashCommandSource extends RemoteFlowSource {
@@ -327,6 +357,22 @@ class Xt0rtedSlashCommandSource extends RemoteFlowSource {
   }
 
   override string getSourceType() { result = "text" }
+
+  override Event getEvent() { result = this.asExpr().getATriggerEvent() }
+}
+
+class ZenteredIssueFormBodyParserSource extends RemoteFlowSource {
+  ZenteredIssueFormBodyParserSource() {
+    exists(UsesStep u |
+      u.getCallee() = "zentered/issue-forms-body-parser" and
+      not exists(u.getArgument("body")) and
+      this.asExpr() = u
+    )
+  }
+
+  override string getSourceType() { result = "text" }
+
+  override Event getEvent() { result = this.asExpr().getATriggerEvent() }
 }
 
 class OctokitRequestActionSource extends RemoteFlowSource {
@@ -348,4 +394,6 @@ class OctokitRequestActionSource extends RemoteFlowSource {
   }
 
   override string getSourceType() { result = "text" }
+
+  override Event getEvent() { result = this.asExpr().getATriggerEvent() }
 }