Merge pull request #103 from github/new_events

Add workflow_dispatch and scheduled to the list of privileged and external (user interaction) events

Author: Alvaro Muñoz

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

@@ -40,9 +40,10 @@ class GitHubCtxSource extends RemoteFlowSource {
 
 class GitHubEventCtxSource extends RemoteFlowSource {
   string flag;
+  string context;
 
   GitHubEventCtxSource() {
-    exists(Expression e, string context, string regexp |
+    exists(Expression e, string regexp |
       this.asExpr() = e and
       context = e.getExpression() and
       (
@@ -62,6 +63,8 @@ class GitHubEventCtxSource extends RemoteFlowSource {
   }
 
   override string getSourceType() { result = flag }
+
+  string getContext() { result = context }
 }
 
 abstract class CommandSource extends RemoteFlowSource {

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

@@ -197,9 +197,23 @@ class ActionsMutableRefCheckout extends MutableRefCheckoutStep instanceof UsesSt
   ActionsMutableRefCheckout() {
     this.getCallee() = "actions/checkout" and
     (
-      exists(ActionsMutableRefCheckoutFlow::PathNode sink |
-        ActionsMutableRefCheckoutFlow::flowPath(_, sink) and
-        sink.getNode().asExpr() = this.getArgumentExpr(["ref", "repository"])
+      exists(
+        ActionsMutableRefCheckoutFlow::PathNode source, ActionsMutableRefCheckoutFlow::PathNode sink
+      |
+        ActionsMutableRefCheckoutFlow::flowPath(source, sink) and
+        sink.getNode().asExpr() = this.getArgumentExpr(["ref", "repository"]) and
+        (
+          not source.getNode() instanceof GitHubEventCtxSource
+          or
+          source.getNode() instanceof GitHubEventCtxSource and
+          // the context is available for the job trigger events
+          exists(string context, string context_prefix |
+            contextTriggerDataModel(this.getEnclosingWorkflow().getATriggerEvent().getName(),
+              context_prefix) and
+            context = source.getNode().(GitHubEventCtxSource).getContext() and
+            normalizeExpr(context).matches("%" + context_prefix + "%")
+          )
+        )
       )
       or
       // heuristic base on the step id and field name
@@ -241,9 +255,21 @@ class ActionsSHACheckout extends SHACheckoutStep instanceof UsesStep {
   ActionsSHACheckout() {
     this.getCallee() = "actions/checkout" and
     (
-      exists(ActionsSHACheckoutFlow::PathNode sink |
-        ActionsSHACheckoutFlow::flowPath(_, sink) and
-        sink.getNode().asExpr() = this.getArgumentExpr(["ref", "repository"])
+      exists(ActionsSHACheckoutFlow::PathNode source, ActionsSHACheckoutFlow::PathNode sink |
+        ActionsSHACheckoutFlow::flowPath(source, sink) and
+        sink.getNode().asExpr() = this.getArgumentExpr(["ref", "repository"]) and
+        (
+          not source.getNode() instanceof GitHubEventCtxSource
+          or
+          source.getNode() instanceof GitHubEventCtxSource and
+          // the context is available for the job trigger events
+          exists(string context, string context_prefix |
+            contextTriggerDataModel(this.getEnclosingWorkflow().getATriggerEvent().getName(),
+              context_prefix) and
+            context = source.getNode().(GitHubEventCtxSource).getContext() and
+            normalizeExpr(context).matches("%" + context_prefix + "%")
+          )
+        )
       )
       or
       // heuristic base on the step id and field name

ql/lib/ext/config/context_event_map.yml

@@ -40,8 +40,6 @@ extensions:
       - ["push", "github.event.commits"]
       - ["push", "github.event.head_commit"]
       - ["push", "github.event.changes"] 
-      - ["repository_dispatch", "github.event.client_payload"]
-      - ["workflow_dispatch", "github.event.inputs"]
       - ["workflow_run", "github.event.workflow"]
       - ["workflow_run", "github.event.workflow_run"]
       - ["workflow_run", "github.event.changes"] 

ql/lib/ext/config/externally_triggereable_events.yml

@@ -16,4 +16,5 @@ extensions:
       - ["pull_request_target"]
       - ["workflow_run"] # depending on branch filter
       - ["workflow_call"] # depending on caller
-
+      - ["workflow_dispatch"] 
+      - ["scheduled"]

ql/lib/ext/config/untrusted_event_properties.yml

@@ -24,8 +24,6 @@ extensions:
       - ["github\\.event\\.workflow_run\\.head_commit\\.message", "text"]
       - ["github\\.event\\.pull_request\\.head\\.repo\\.description", "text"]
       - ["github\\.event\\.workflow_run\\.head_repository\\.description", "text"]
-      - ["github\\.event\\.client_payload\\[[0-9]+\\]", "text"]
-      - ["github\\.event\\.client_payload", "text"]
       - ["github\\.event\\.changes\\.body\\.from", "title"]
       # BRANCH
       - ["github\\.event\\.pull_request\\.head\\.repo\\.default_branch", "branch"]
@@ -59,7 +57,6 @@ extensions:
       # JSON
       - ["github", "json"]
       - ["github\\.event", "json"]
-      - ["github\\.event\\.client_payload", "json"]
       - ["github\\.event\\.comment", "json"]
       - ["github\\.event\\.commits", "json"]
       - ["github\\.event\\.discussion", "json"]

ql/src/Security/CWE-829/UntrustedCheckoutCritical.ql

@@ -47,6 +47,8 @@ where
   ) and
   // the checkout occurs in a privileged context
   inPrivilegedContext(poisonable, event) and
+  not exists(ControlCheck check | check.protects(checkout, event, "untrusted-checkout")) and
   not exists(ControlCheck check | check.protects(poisonable, event, "untrusted-checkout"))
-select poisonable, checkout, poisonable, "Execution of untrusted code on a privileged workflow. $@",
-  event, event.getLocation().getFile().toString()
+select poisonable, checkout, poisonable,
+  "Execution of untrusted code on a privileged workflow ($@)", event,
+  event.getLocation().getFile().toString()

ql/test/query-tests/Security/CWE-078/CommandInjectionCritical.expected

@@ -1,13 +1,6 @@
 edges
-| .github/actions/run-airbyte-ci/action.yaml:4:3:4:12 | input subcommand | .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | provenance |  |
-| .github/workflows/test1.yml:46:42:46:90 | github.event.client_payload.connector_name | .github/actions/run-airbyte-ci/action.yaml:4:3:4:12 | input subcommand | provenance |  |
-| .github/workflows/test1.yml:63:42:63:90 | github.event.client_payload.connector_name | .github/actions/run-airbyte-ci/action.yaml:4:3:4:12 | input subcommand | provenance |  |
 nodes
-| .github/actions/run-airbyte-ci/action.yaml:4:3:4:12 | input subcommand | semmle.label | input subcommand |
-| .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | semmle.label | inputs.subcommand |
 | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | semmle.label | github.event.comment.body |
-| .github/workflows/test1.yml:46:42:46:90 | github.event.client_payload.connector_name | semmle.label | github.event.client_payload.connector_name |
-| .github/workflows/test1.yml:63:42:63:90 | github.event.client_payload.connector_name | semmle.label | github.event.client_payload.connector_name |
 subpaths
 #select
 | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | Potential command injection in $@, which may be controlled by an external user. | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | ${{ github.event.comment.body }} |

ql/test/query-tests/Security/CWE-078/CommandInjectionMedium.expected

@@ -1,14 +1,5 @@
 edges
-| .github/actions/run-airbyte-ci/action.yaml:4:3:4:12 | input subcommand | .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | provenance |  |
-| .github/workflows/test1.yml:46:42:46:90 | github.event.client_payload.connector_name | .github/actions/run-airbyte-ci/action.yaml:4:3:4:12 | input subcommand | provenance |  |
-| .github/workflows/test1.yml:63:42:63:90 | github.event.client_payload.connector_name | .github/actions/run-airbyte-ci/action.yaml:4:3:4:12 | input subcommand | provenance |  |
 nodes
-| .github/actions/run-airbyte-ci/action.yaml:4:3:4:12 | input subcommand | semmle.label | input subcommand |
-| .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | semmle.label | inputs.subcommand |
 | .github/workflows/comment_issue.yml:9:26:9:57 | github.event.comment.body | semmle.label | github.event.comment.body |
-| .github/workflows/test1.yml:46:42:46:90 | github.event.client_payload.connector_name | semmle.label | github.event.client_payload.connector_name |
-| .github/workflows/test1.yml:63:42:63:90 | github.event.client_payload.connector_name | semmle.label | github.event.client_payload.connector_name |
 subpaths
 #select
-| .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | .github/workflows/test1.yml:46:42:46:90 | github.event.client_payload.connector_name | .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | Potential command injection in $@, which may be controlled by an external user. | .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | ${{ inputs.subcommand }} |
-| .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | .github/workflows/test1.yml:63:42:63:90 | github.event.client_payload.connector_name | .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | Potential command injection in $@, which may be controlled by an external user. | .github/actions/run-airbyte-ci/action.yaml:163:118:163:141 | inputs.subcommand | ${{ inputs.subcommand }} |

ql/test/query-tests/Security/CWE-094/.github/workflows/test18.yml

@@ -0,0 +1,33 @@
+on:
+  workflow_dispatch:
+
+jobs:
+  fetch-issues:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Fetch open issues
+        id: issues
+        uses: octokit/request-action@v2.x
+        with:
+          route: GET /repos/foo/bar/issues?state=open
+        env:
+          GITHUB_TOKEN: ${{ secrets.GITHUBACTIONS_TOKEN }}
+
+      - name: Write issues to file
+        run: |
+          echo '${{ steps.issues.outputs.data }}' > issues.json
+          
+      - name: Setup Node.js
+        uses: actions/setup-node@v2
+        with:
+          node-version: '14'
+
+      - name: Print issue URLs
+        run: |
+          const fs = require('fs');
+          const issues = JSON.parse(fs.readFileSync('issues.json', 'utf8'));
+          const filteredIssues = issues.filter(issue => issue.body.includes('Is your portal managed or self-hosted?\r\n\r\nManaged'));
+          for (const issue of filteredIssues) {
+            console.log(issue.html_url);
+          }
+        shell: bash

ql/test/query-tests/Security/CWE-094/CodeInjectionCritical.expected

@@ -159,6 +159,7 @@ edges
 | .github/workflows/test17.yml:30:13:39:10 | Uses Step: get-pr-details | .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | provenance |  |
 | .github/workflows/test17.yml:49:13:55:10 | Uses Step: issues | .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | provenance |  |
 | .github/workflows/test17.yml:60:13:68:10 | Uses Step: get-pull-request | .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | provenance |  |
+| .github/workflows/test18.yml:8:9:16:6 | Uses Step: issues | .github/workflows/test18.yml:18:18:18:49 | steps.issues.outputs.data | provenance |  |
 | .github/workflows/test.yml:11:7:13:4 | Job outputs node [job_output] | .github/workflows/test.yml:52:20:52:56 | needs.job1.outputs['job_output'] | provenance |  |
 | .github/workflows/test.yml:11:20:11:50 | steps.step5.outputs.MSG5 | .github/workflows/test.yml:11:7:13:4 | Job outputs node [job_output] | provenance |  |
 | .github/workflows/test.yml:17:9:23:6 | Uses Step: step0 [value] | .github/workflows/test.yml:25:18:25:48 | steps.step0.outputs.value | provenance |  |
@@ -484,6 +485,8 @@ nodes
 | .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | semmle.label | steps.issues.outputs.data |
 | .github/workflows/test17.yml:60:13:68:10 | Uses Step: get-pull-request | semmle.label | Uses Step: get-pull-request |
 | .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | semmle.label | fromJson(steps.get-pull-request.outputs.data).title |
+| .github/workflows/test18.yml:8:9:16:6 | Uses Step: issues | semmle.label | Uses Step: issues |
+| .github/workflows/test18.yml:18:18:18:49 | steps.issues.outputs.data | semmle.label | steps.issues.outputs.data |
 | .github/workflows/test.yml:11:7:13:4 | Job outputs node [job_output] | semmle.label | Job outputs node [job_output] |
 | .github/workflows/test.yml:11:20:11:50 | steps.step5.outputs.MSG5 | semmle.label | steps.step5.outputs.MSG5 |
 | .github/workflows/test.yml:17:9:23:6 | Uses Step: step0 [value] | semmle.label | Uses Step: step0 [value] |
@@ -639,6 +642,7 @@ subpaths
 | .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | .github/workflows/test17.yml:30:13:39:10 | Uses Step: get-pr-details | .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test17.yml:45:30:45:88 | fromJson(steps.get-pr-details.outputs.data).head.ref | ${{ fromJson(steps.get-pr-details.outputs.data).head.ref }} |
 | .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | .github/workflows/test17.yml:49:13:55:10 | Uses Step: issues | .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test17.yml:56:22:56:53 | steps.issues.outputs.data | ${{ steps.issues.outputs.data }} |
 | .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | .github/workflows/test17.yml:60:13:68:10 | Uses Step: get-pull-request | .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test17.yml:69:13:71:55 | fromJson(steps.get-pull-request.outputs.data).title | ${{ fromJson(steps.get-pull-request.outputs.data).title }} |
+| .github/workflows/test18.yml:18:18:18:49 | steps.issues.outputs.data | .github/workflows/test18.yml:8:9:16:6 | Uses Step: issues | .github/workflows/test18.yml:18:18:18:49 | steps.issues.outputs.data | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test18.yml:18:18:18:49 | steps.issues.outputs.data | ${{ steps.issues.outputs.data }} |
 | .github/workflows/test.yml:52:20:52:56 | needs.job1.outputs['job_output'] | .github/workflows/test.yml:20:20:20:62 | github.event['pull_request']['body'] | .github/workflows/test.yml:52:20:52:56 | needs.job1.outputs['job_output'] | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/test.yml:52:20:52:56 | needs.job1.outputs['job_output'] | ${{needs.job1.outputs['job_output']}} |
 | .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | .github/workflows/untrusted_checkout1.yml:8:9:11:6 | Uses Step | .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/untrusted_checkout1.yml:15:20:15:58 | steps.artifact.outputs.pr_number | ${{ steps.artifact.outputs.pr_number }} |
 | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/workflow_run.yml:9:19:9:64 | github.event.workflow_run.display_title | ${{ github.event.workflow_run.display_title }} |