Split sources by taint type

Author: Alvaro Muñoz

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

@@ -1,5 +1,3 @@
-private import actions
-private import codeql.actions.DataFlow
 private import codeql.actions.dataflow.ExternalFlow
 private import codeql.actions.security.ArtifactPoisoningQuery
 
@@ -22,152 +20,220 @@ abstract class RemoteFlowSource extends SourceNode {
 }
 
 bindingset[context]
-private predicate isExternalUserControlled(string context) {
-  exists(string reg | reg = "github\\.event" |
+private predicate titleEvent(string context) {
+  exists(string reg |
+    reg =
+      [
+        // title
+        "github\\.event\\.issue\\.title", // issue
+        "github\\.event\\.pull_request\\.title", // pull request
+        "github\\.event\\.discussion\\.title", // discussion
+        "github\\.event\\.pages\\[[0-9]+\\]\\.page_name",
+        "github\\.event\\.pages\\[[0-9]+\\]\\.title",
+        "github\\.event\\.workflow_run\\.display_title", // The event-specific title associated with the run or the run-name if set, or the value of run-name if it is set in the workflow.
+      ]
+  |
     Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
   )
 }
 
 bindingset[context]
-private predicate isExternalUserControlledIssue(string context) {
-  exists(string reg | reg = ["github\\.event\\.issue\\.title", "github\\.event\\.issue\\.body"] |
+private predicate urlEvent(string context) {
+  exists(string reg |
+    reg =
+      [
+        // url
+        "github\\.event\\.pull_request\\.head\\.repo\\.homepage",
+      ]
+  |
     Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
   )
 }
 
 bindingset[context]
-private predicate isExternalUserControlledPullRequest(string context) {
+private predicate textEvent(string context) {
   exists(string reg |
     reg =
       [
-        "github\\.event\\.pull_request\\.title", "github\\.event\\.pull_request\\.body",
-        "github\\.event\\.pull_request\\.head\\.label",
-        "github\\.event\\.pull_request\\.head\\.repo\\.default_branch",
-        "github\\.event\\.pull_request\\.head\\.repo\\.description",
-        "github\\.event\\.pull_request\\.head\\.repo\\.homepage",
-        "github\\.event\\.pull_request\\.head\\.ref", "github\\.head_ref"
+        // text
+        "github\\.event\\.issue\\.body", // body
+        "github\\.event\\.pull_request\\.body", // body
+        "github\\.event\\.discussion\\.body", // body
+        "github\\.event\\.review\\.body", // body
+        "github\\.event\\.comment\\.body", // body
+        "github\\.event\\.commits\\[[0-9]+\\]\\.message", // messsage
+        "github\\.event\\.head_commit\\.message", // message
+        "github\\.event\\.workflow_run\\.head_commit\\.message", // message
+        "github\\.event\\.pull_request\\.head\\.repo\\.description", // description
+        "github\\.event\\.workflow_run\\.head_repository\\.description", // description
+        "github\\.event\\.client_payload\\[[0-9]+\\]", // payload
+        "github\\.event\\.client_payload", // payload
+        "github\\.event\\.inputs\\[[0-9]+\\]", // input
+        "github\\.event\\.inputs", // input
       ]
   |
     Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
   )
 }
 
 bindingset[context]
-private predicate isExternalUserControlledReview(string context) {
-  Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp("github\\.event\\.review\\.body"))
+private predicate repoNameEvent(string context) {
+  exists(string reg |
+    reg =
+      [
+        // repo name
+        // Owner: All characters must be either a hyphen (-) or alphanumeric
+        // Repo: All code points must be either a hyphen (-), an underscore (_), a period (.), or an ASCII alphanumeric code point
+        "github\\.event\\.workflow_run\\.pull_requests\\[[0-9]+\\]\\.head\\.repo\\.name", // repo name
+        "github\\.event\\.workflow_run\\.head_repository\\.name", // repo name
+        "github\\.event\\.workflow_run\\.head_repository\\.full_name", // nwo
+      ]
+  |
+    Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
+  )
 }
 
 bindingset[context]
-private predicate isExternalUserControlledComment(string context) {
-  Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp("github\\.event\\.comment\\.body"))
+private predicate branchEvent(string context) {
+  exists(string reg |
+    reg =
+      [
+        // branch
+        // https://docs.github.com/en/get-started/using-git/dealing-with-special-characters-in-branch-and-tag-names
+        // - They can include slash / for hierarchical (directory) grouping, but no slash-separated component can begin with a dot . or end with the sequence .lock.
+        // - They must contain at least one /
+        // - They cannot have two consecutive dots .. anywhere.
+        // - They cannot have ASCII control characters (i.e. bytes whose values are lower than \040, or \177 DEL), space, tilde ~, caret ^, or colon : anywhere.
+        // - They cannot have question-mark ?, asterisk *, or open bracket [ anywhere.
+        // - They cannot begin or end with a slash / or contain multiple consecutive slashes
+        // - They cannot end with a dot .
+        // - They cannot contain a sequence @{
+        // - They cannot be the single character @
+        // - They cannot contain a \
+        // eg: zzz";echo${IFS}"hello";# would be a valid branch name
+        "github\\.event\\.pull_request\\.head\\.repo\\.default_branch",
+        "github\\.event\\.pull_request\\.head\\.ref", "github\\.head_ref",
+        "github\\.event\\.workflow_run\\.head_branch",
+        "github\\.event\\.workflow_run\\.head_branch",
+        "github\\.event\\.workflow_run\\.pull_requests\\[[0-9]+\\]\\.head\\.ref",
+      ]
+  |
+    Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
+  )
 }
 
 bindingset[context]
-private predicate isExternalUserControlledGollum(string context) {
+private predicate labelEvent(string context) {
   exists(string reg |
     reg =
       [
-        "github\\.event\\.pages\\[[0-9]+\\]\\.page_name",
-        "github\\.event\\.pages\\[[0-9]+\\]\\.title"
+        // label
+        // - They cannot contain a escaping \
+        "github\\.event\\.pull_request\\.head\\.label",
       ]
   |
     Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
   )
 }
 
 bindingset[context]
-private predicate isExternalUserControlledCommit(string context) {
+private predicate emailEvent(string context) {
   exists(string reg |
     reg =
       [
-        "github\\.event\\.commits\\[[0-9]+\\]\\.message", "github\\.event\\.head_commit\\.message",
+        // email
+        // `echo${IFS}hello`@domain.com
         "github\\.event\\.head_commit\\.author\\.email",
-        "github\\.event\\.head_commit\\.author\\.name",
         "github\\.event\\.head_commit\\.committer\\.email",
-        "github\\.event\\.head_commit\\.committer\\.name",
         "github\\.event\\.commits\\[[0-9]+\\]\\.author\\.email",
-        "github\\.event\\.commits\\[[0-9]+\\]\\.author\\.name",
         "github\\.event\\.commits\\[[0-9]+\\]\\.committer\\.email",
-        "github\\.event\\.commits\\[[0-9]+\\]\\.committer\\.name",
+        "github\\.event\\.workflow_run\\.head_commit\\.author\\.email",
+        "github\\.event\\.workflow_run\\.head_commit\\.committer\\.email",
       ]
   |
     Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
   )
 }
 
 bindingset[context]
-private predicate isExternalUserControlledDiscussion(string context) {
+private predicate usernameEvent(string context) {
   exists(string reg |
-    reg = ["github\\.event\\.discussion\\.title", "github\\.event\\.discussion\\.body"]
+    reg =
+      [
+        // username
+        // All characters must be either a hyphen (-) or alphanumeric
+        "github\\.event\\.head_commit\\.author\\.name",
+        "github\\.event\\.head_commit\\.committer\\.name",
+        "github\\.event\\.commits\\[[0-9]+\\]\\.author\\.name",
+        "github\\.event\\.commits\\[[0-9]+\\]\\.committer\\.name",
+        "github\\.event\\.workflow_run\\.head_commit\\.author\\.name",
+        "github\\.event\\.workflow_run\\.head_commit\\.committer\\.name",
+      ]
   |
     Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
   )
 }
 
 bindingset[context]
-private predicate isExternalUserControlledWorkflowRun(string context) {
+private predicate pathEvent(string context) {
   exists(string reg |
     reg =
       [
-        "github\\.event\\.workflow\\.path", "github\\.event\\.workflow_run\\.head_branch",
-        "github\\.event\\.workflow_run\\.display_title",
-        "github\\.event\\.workflow_run\\.head_branch",
-        "github\\.event\\.workflow_run\\.head_repository\\.description",
-        "github\\.event\\.workflow_run\\.head_repository\\.full_name",
-        "github\\.event\\.workflow_run\\.head_repository\\.name",
-        "github\\.event\\.workflow_run\\.head_commit\\.message",
-        "github\\.event\\.workflow_run\\.head_commit\\.author\\.email",
-        "github\\.event\\.workflow_run\\.head_commit\\.author\\.name",
-        "github\\.event\\.workflow_run\\.head_commit\\.committer\\.email",
-        "github\\.event\\.workflow_run\\.head_commit\\.committer\\.name",
-        "github\\.event\\.workflow_run\\.pull_requests\\[[0-9]+\\]\\.head\\.ref",
-        "github\\.event\\.workflow_run\\.pull_requests\\[[0-9]+\\]\\.head\\.repo\\.name",
+        // filename
+        "github\\.event\\.workflow\\.path",
       ]
   |
     Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
   )
 }
 
 bindingset[context]
-private predicate isExternalUserControlledRepositoryDispatch(string context) {
+private predicate jsonEvent(string context) {
   exists(string reg |
-    reg = ["github\\.event\\.client_payload\\[[0-9]+\\]", "github\\.event\\.client_payload",]
+    reg =
+      [
+        // json
+        "github\\.event",
+      ]
   |
     Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
   )
 }
 
-bindingset[context]
-private predicate isExternalUserControlledWorkflowDispatch(string context) {
-  exists(string reg | reg = ["github\\.event\\.inputs\\[[0-9]+\\]", "github\\.event\\.inputs",] |
-    Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
-  )
-}
+class EventSource extends RemoteFlowSource {
+  string flag;
 
-private class EventSource extends RemoteFlowSource {
   EventSource() {
     exists(Expression e, string context | this.asExpr() = e and context = e.getExpression() |
-      isExternalUserControlled(context) or
-      isExternalUserControlledIssue(context) or
-      isExternalUserControlledPullRequest(context) or
-      isExternalUserControlledReview(context) or
-      isExternalUserControlledComment(context) or
-      isExternalUserControlledGollum(context) or
-      isExternalUserControlledCommit(context) or
-      isExternalUserControlledDiscussion(context) or
-      isExternalUserControlledWorkflowRun(context) or
-      isExternalUserControlledRepositoryDispatch(context) or
-      isExternalUserControlledWorkflowDispatch(context)
+      titleEvent(context) and flag = "title"
+      or
+      urlEvent(context) and flag = "url"
+      or
+      textEvent(context) and flag = "text"
+      or
+      repoNameEvent(context) and flag = "repo name"
+      or
+      branchEvent(context) and flag = "branch"
+      or
+      labelEvent(context) and flag = "label"
+      or
+      emailEvent(context) and flag = "email"
+      or
+      usernameEvent(context) and flag = "username"
+      or
+      pathEvent(context) and flag = "filename"
+      or
+      jsonEvent(context) and flag = "json"
     )
   }
 
-  override string getSourceType() { result = "User-controlled events" }
+  override string getSourceType() { result = flag }
 }
 
 /**
  * A Source of untrusted data defined in a MaD specification
  */
-private class ExternallyDefinedSource extends RemoteFlowSource {
+class ExternallyDefinedSource extends RemoteFlowSource {
   string sourceType;
 
   ExternallyDefinedSource() { externallyDefinedSource(this, sourceType, _) }
@@ -178,19 +244,19 @@ private class ExternallyDefinedSource extends RemoteFlowSource {
 /**
  * An input for a Composite Action
  */
-private class CompositeActionInputSource extends RemoteFlowSource {
+class CompositeActionInputSource extends RemoteFlowSource {
   CompositeAction c;
 
   CompositeActionInputSource() { c.getAnInput() = this.asExpr() }
 
-  override string getSourceType() { result = "Composite action input" }
+  override string getSourceType() { result = "input" }
 }
 
 /**
  * A downloadeded artifact.
  */
-private class ArtifactToOptionSource extends RemoteFlowSource {
-  ArtifactToOptionSource() { this.asExpr() instanceof UntrustedArtifactDownloadStep }
+private class ArtifactSource extends RemoteFlowSource {
+  ArtifactSource() { this.asExpr() instanceof UntrustedArtifactDownloadStep }
 
-  override string getSourceType() { result = "Step output from Artifact" }
+  override string getSourceType() { result = "artifact" }
 }

ql/lib/codeql/actions/security/EnvVarInjectionQuery.qll

@@ -7,14 +7,6 @@ import codeql.actions.DataFlow
 
 abstract class EnvVarInjectionSink extends DataFlow::Node { }
 
-// predicate envVarInjectionFromEnvVarSink(DataFlow::Node sink) {
-//   exists(Expression expr, Run run, string varName, string key, string value |
-//     expr = run.getInScopeEnvVarExpr(varName) and
-//     Utils::writeToGitHubEnv(run, key, value) and
-//     expr = sink.asExpr() and
-//     value.matches("%$" + ["", "{", "ENV{"] + varName + "%")
-//   )
-// }
 class EnvVarInjectionFromEnvVarSink extends EnvVarInjectionSink {
   EnvVarInjectionFromEnvVarSink() {
     exists(Run run, Expression expr, string varname, string key, string value |

ql/lib/ext/ahmadnassri_action-changed-files.model.yml

@@ -3,5 +3,5 @@ extensions:
       pack: githubsecuritylab/actions-all
       extensible: sourceModel
     data:
-      - ["ahmadnassri/action-changed-files", "*", "output.files", "PR changed files", "manual"]
-      - ["ahmadnassri/action-changed-files", "*", "output.json", "PR changed files", "manual"]
+      - ["ahmadnassri/action-changed-files", "*", "output.files", "filename", "manual"]
+      - ["ahmadnassri/action-changed-files", "*", "output.json", "json", "manual"]

ql/lib/ext/amannn_action-semantic-pull-request.model.yml

@@ -3,4 +3,4 @@ extensions:
       pack: githubsecuritylab/actions-all
       extensible: sourceModel
     data:
-      - ["amannn/action-semantic-pull-request", "*", "output.error_message", "PR title", "manual"]
+      - ["amannn/action-semantic-pull-request", "*", "output.error_message", "text", "manual"]

ql/lib/ext/cypress-io_github-action.model.yml

@@ -3,4 +3,4 @@ extensions:
       pack: githubsecuritylab/actions-all
       extensible: sourceModel
     data:
-      - ["cypress-io/github-action", "*", "env.GH_BRANCH", "PR branch", "manual"]
+      - ["cypress-io/github-action", "*", "env.GH_BRANCH", "branch", "manual"]

ql/lib/ext/dawidd6_action-download-artifact.model.yml

@@ -3,4 +3,4 @@ extensions:
       pack: githubsecuritylab/actions-all
       extensible: sourceModel
     data:
-      - ["dawidd6/action-download-artifact", "*", "output.artifacts", "Artifact details", "manual"]
+      - ["dawidd6/action-download-artifact", "*", "output.artifacts", "artifact", "manual"]

ql/lib/ext/dorny_paths-filter.model.yml

@@ -3,4 +3,4 @@ extensions:
       pack: githubsecuritylab/actions-all
       extensible: sourceModel
     data:
-      - ["dorny/paths-filter", "*", "output.changes", "PR changed files", "manual"]
+      - ["dorny/paths-filter", "*", "output.changes", "filename", "manual"]

ql/lib/ext/franzdiebold_github-env-vars-action.model.yml

@@ -3,5 +3,5 @@ extensions:
       pack: githubsecuritylab/actions-all
       extensible: sourceModel
     data:
-      - ["franzdiebold/github-env-vars-action", "*", "output.CI_PR_DESCRIPTION", "PR body", "manual"]
-      - ["franzdiebold/github-env-vars-action", "*", "output.CI_PR_TITLE", "PR title", "manual"]
+      - ["franzdiebold/github-env-vars-action", "*", "output.CI_PR_DESCRIPTION", "text", "manual"]
+      - ["franzdiebold/github-env-vars-action", "*", "output.CI_PR_TITLE", "title", "manual"]

ql/lib/ext/generated/composite-actions/googlecloudplatform_dataflowtemplates.model.yml

@@ -8,4 +8,4 @@ extensions:
       pack: githubsecuritylab/actions-all
       extensible: sourceModel
     data:
-     - ["googlecloudplatform/magic-modules", "*", "output.changed-files", "PR changed files", "manual"]
+     - ["googlecloudplatform/magic-modules", "*", "output.changed-files", "filename", "manual"]

ql/lib/ext/generated/reusable-workflows/puppeteer_puppeteer.model.yml

@@ -3,4 +3,4 @@ extensions:
       pack: githubsecuritylab/actions-all
       extensible: sourceModel
     data:
-      - ["puppeteer/puppeteer/.github/workflows/changed-packages.yml", "*", "output.changes", "Changed files", "manual"]
+      - ["puppeteer/puppeteer/.github/workflows/changed-packages.yml", "*", "output.changes", "filename", "manual"]