feat(queries): Improve Output Clobbering query

Add support for clobbering of `set-output` workflow command
2025-12-27 22:26:31 +01:00 · 2024-08-07 13:17:36 +02:00
parent c442f1b96b
commit 473251371b
6 changed files with 147 additions and 8 deletions
--- a/ql/lib/codeql/actions/Helper.qll
+++ b/ql/lib/codeql/actions/Helper.qll
@@ -20,7 +20,7 @@ string wrapJsonRegexp(string regex) {
 }

 bindingset[str]
-private string trimQuotes(string str) {
+string trimQuotes(string str) {
  result = str.trim().regexpReplaceAll("^(\"|')", "").regexpReplaceAll("(\"|')$", "")
 }

@@ -279,6 +279,10 @@ predicate inNonPrivilegedContext(AstNode node) {
  inNonPrivilegedJob(node)
 }

+string partialFileContentRegexp() {
+  result = ["cat\\s+", "jq\\s+", "yq\\s+", "tail\\s+", "head\\s+", "ls\\s+"]
+}
+
 bindingset[snippet]
 predicate outputsPartialFileContent(string snippet) {
  // e.g.
@@ -286,12 +290,7 @@ predicate outputsPartialFileContent(string snippet) {
  // echo "FOO=$(<foo.txt)" >> $GITHUB_ENV
  // yq '.foo' foo.yml >> $GITHUB_PATH
  // cat foo.txt >> $GITHUB_PATH
-  snippet
-      .regexpMatch([
-          "(\\$\\(|`)<.*",
-          ".*(\\b|^|\\s+)" + ["cat\\s+", "jq\\s+", "yq\\s+", "tail\\s+", "head\\s+", "ls\\s+"] +
-            ".*"
-        ])
+  snippet.regexpMatch(["(\\$\\(|`)<.*", ".*(\\b|^|\\s+)" + partialFileContentRegexp() + ".*"])
 }

 string defaultBranchNames() {
--- a/ql/lib/codeql/actions/dataflow/FlowSteps.qll
+++ b/ql/lib/codeql/actions/dataflow/FlowSteps.qll
@@ -8,6 +8,7 @@ private import codeql.actions.DataFlow
 private import codeql.actions.dataflow.FlowSources
 private import codeql.actions.dataflow.ExternalFlow
 private import codeql.actions.security.ArtifactPoisoningQuery
+private import codeql.actions.security.OutputClobberingQuery
 private import codeql.actions.security.UntrustedCheckoutQuery

 /**
@@ -114,7 +115,8 @@ predicate envToRunStep(DataFlow::Node pred, DataFlow::Node succ) {
    succ.asExpr() = run.getScriptScalar() and
    (
      envToSpecialFile(["GITHUB_ENV", "GITHUB_OUTPUT", "GITHUB_PATH"], var_name, run, _) or
-      envToArgInjSink(var_name, run, _)
+      envToArgInjSink(var_name, run, _) or
+      exists(OutputClobberingSink n | n.asExpr() = run.getScriptScalar())
    )
  )
 }
--- a/ql/lib/codeql/actions/security/OutputClobberingQuery.qll
+++ b/ql/lib/codeql/actions/security/OutputClobberingQuery.qll
@@ -92,6 +92,69 @@ class OutputClobberingFromEnvVarSink extends OutputClobberingSink {
  }
 }

+/**
+ *      - id: clob1
+ *        env:
+ *          BODY: ${{ github.event.comment.body }}
+ *        run: |
+ *          # VULNERABLE
+ *          echo $BODY
+ *          echo "::set-output name=OUTPUT::SAFE"
+ *      - id: clob2
+ *        env:
+ *          BODY: ${{ github.event.comment.body }}
+ *        run: |
+ *          # VULNERABLE
+ *          echo "::set-output name=OUTPUT::SAFE"
+ *          echo $BODY
+ */
+class WorkflowCommandClobberingFromEnvVarSink extends OutputClobberingSink {
+  WorkflowCommandClobberingFromEnvVarSink() {
+    exists(Run run, string output_line, string clobbering_line, string var_name |
+      run.getScript().splitAt("\n") = output_line and
+      singleLineWorkflowCmd(output_line, "set-output", _, _) and
+      run.getScript().splitAt("\n") = clobbering_line and
+      clobbering_line.regexpMatch(".*echo\\s+(-e\\s+)?(\"|')?\\$(\\{)?" + var_name + ".*") and
+      exists(run.getInScopeEnvVarExpr(var_name)) and
+      run.getScriptScalar() = this.asExpr()
+    )
+  }
+}
+
+class WorkflowCommandClobberingFromFileReadSink extends OutputClobberingSink {
+  WorkflowCommandClobberingFromFileReadSink() {
+    exists(Run run, string output_line, string clobbering_line |
+      run.getScriptScalar() = this.asExpr() and
+      run.getScript().splitAt("\n") = output_line and
+      singleLineWorkflowCmd(output_line, "set-output", _, _) and
+      run.getScript().splitAt("\n") = clobbering_line and
+      (
+        // A file is read and its content is assigned to an env var that gets printed to stdout
+        // - run: |
+        //     foo=$(<pr-id.txt)"
+        //     echo "${foo}"
+        exists(string var_name, string value, string assign_line, string assignment_regexp |
+          run.getScript().splitAt("\n") = assign_line and
+          assignment_regexp = "([a-zA-Z0-9\\-_]+)=(.*)" and
+          var_name = assign_line.regexpCapture(assignment_regexp, 1) and
+          value = assign_line.regexpCapture(assignment_regexp, 2) and
+          outputsPartialFileContent(trimQuotes(value)) and
+          clobbering_line.regexpMatch(".*echo\\s+(-e\\s+)?(\"|')?\\$(\\{)?" + var_name + ".*")
+        )
+        or
+        // A file is read and its content is printed to stdout
+        // - run: echo "foo=$(<pr-id.txt)"
+        clobbering_line.regexpMatch(".*echo\\s+(-e)?\\s*(\"|')?") and
+        clobbering_line.regexpMatch(partialFileContentRegexp() + ".*")
+        or
+        // A file content is printed to stdout
+        // - run: cat pr-id.txt
+        clobbering_line.regexpMatch(partialFileContentRegexp() + ".*")
+      )
+    )
+  }
+}
+
 class OutputClobberingFromMaDSink extends OutputClobberingSink {
  OutputClobberingFromMaDSink() { madSink(this, "output-clobbering") }
 }
--- a/ql/src/Security/CWE-077/OutputClobberingHigh.ql
+++ b/ql/src/Security/CWE-077/OutputClobberingHigh.ql
@@ -30,6 +30,7 @@ where
    source.getNode().(RemoteFlowSource).getSourceType() = "artifact" and
    (
      sink.getNode() instanceof OutputClobberingFromFileReadSink or
+      sink.getNode() instanceof WorkflowCommandClobberingFromFileReadSink or
      madSink(sink.getNode(), "output-clobbering")
    )
  )
--- a/ql/test/query-tests/Security/CWE-077/.github/workflows/output2.yml
+++ b/ql/test/query-tests/Security/CWE-077/.github/workflows/output2.yml
@@ -0,0 +1,56 @@
+on:
+  issue_comment:
+jobs:
+  test1:
+    runs-on: ubuntu-latest
+    steps:
+      - id: clob1
+        env:
+          BODY: ${{ github.event.comment.body }}
+        run: |
+          # VULNERABLE
+          echo $BODY
+          echo "::set-output name=OUTPUT::SAFE"
+      - id: clob2
+        env:
+          BODY: ${{ github.event.comment.body }}
+        run: |
+          # VULNERABLE
+          echo "::set-output name=OUTPUT::SAFE"
+          echo $BODY
+      - id: clob3
+        run: |
+          echo ${{ steps.clob1.outputs.OUTPUT }}
+  test2:
+    runs-on: ubuntu-latest
+    steps:
+      - id: clob1
+        env:
+          BODY: ${{ github.event.comment.body }}
+        run: |
+          # NOT VULNERABLE
+          echo "::set-output name=OUTPUT::SAFE"
+  test3:
+    runs-on: ubuntu-latest
+    steps:
+      - name: Download artifact
+        uses: dawidd6/action-download-artifact@v6
+        with:
+          run_id: ${{ github.event.workflow_run.id }}
+          name: pr_number 
+      - id: clob1
+        run: |
+          # VULNERABLE
+          PR="$(<pr-number)"
+          echo "$PR"
+          echo "::set-output name=OUTPUT::SAFE"
+      - id: clob2
+        run: |
+          # VULNERABLE
+          cat pr-number
+          echo "::set-output name=OUTPUT::SAFE"
+      - id: clob2
+        run: |
+          # VULNERABLE
+          echo "::set-output name=OUTPUT::SAFE"
+          ls *.txt
--- a/ql/test/query-tests/Security/CWE-077/OutputClobberingHigh.expected
+++ b/ql/test/query-tests/Security/CWE-077/OutputClobberingHigh.expected
@@ -1,12 +1,30 @@
 edges
 | .github/workflows/output1.yml:9:18:9:49 | github.event.comment.body | .github/workflows/output1.yml:10:14:13:50 | # VULNERABLE\necho "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$BODY" >> $GITHUB_OUTPUT\n | provenance |  |
 | .github/workflows/output1.yml:30:9:35:6 | Uses Step | .github/workflows/output1.yml:36:14:38:58 | echo "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$(<pr-number)" >> $GITHUB_OUTPUT\n | provenance |  |
+| .github/workflows/output2.yml:9:18:9:49 | github.event.comment.body | .github/workflows/output2.yml:10:14:13:48 | # VULNERABLE\necho $BODY\necho "::set-output name=OUTPUT::SAFE"\n | provenance |  |
+| .github/workflows/output2.yml:16:18:16:49 | github.event.comment.body | .github/workflows/output2.yml:17:14:20:21 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\necho $BODY\n | provenance |  |
+| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:42:14:46:48 | # VULNERABLE\nPR="$(<pr-number)"\necho "$PR"\necho "::set-output name=OUTPUT::SAFE"\n | provenance |  |
+| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:48:14:51:48 | # VULNERABLE\ncat pr-number\necho "::set-output name=OUTPUT::SAFE"\n | provenance |  |
+| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:53:14:56:19 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\nls *.txt\n | provenance |  |
 nodes
 | .github/workflows/output1.yml:9:18:9:49 | github.event.comment.body | semmle.label | github.event.comment.body |
 | .github/workflows/output1.yml:10:14:13:50 | # VULNERABLE\necho "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$BODY" >> $GITHUB_OUTPUT\n | semmle.label | # VULNERABLE\necho "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$BODY" >> $GITHUB_OUTPUT\n |
 | .github/workflows/output1.yml:30:9:35:6 | Uses Step | semmle.label | Uses Step |
 | .github/workflows/output1.yml:36:14:38:58 | echo "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$(<pr-number)" >> $GITHUB_OUTPUT\n | semmle.label | echo "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$(<pr-number)" >> $GITHUB_OUTPUT\n |
+| .github/workflows/output2.yml:9:18:9:49 | github.event.comment.body | semmle.label | github.event.comment.body |
+| .github/workflows/output2.yml:10:14:13:48 | # VULNERABLE\necho $BODY\necho "::set-output name=OUTPUT::SAFE"\n | semmle.label | # VULNERABLE\necho $BODY\necho "::set-output name=OUTPUT::SAFE"\n |
+| .github/workflows/output2.yml:16:18:16:49 | github.event.comment.body | semmle.label | github.event.comment.body |
+| .github/workflows/output2.yml:17:14:20:21 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\necho $BODY\n | semmle.label | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\necho $BODY\n |
+| .github/workflows/output2.yml:36:9:41:6 | Uses Step | semmle.label | Uses Step |
+| .github/workflows/output2.yml:42:14:46:48 | # VULNERABLE\nPR="$(<pr-number)"\necho "$PR"\necho "::set-output name=OUTPUT::SAFE"\n | semmle.label | # VULNERABLE\nPR="$(<pr-number)"\necho "$PR"\necho "::set-output name=OUTPUT::SAFE"\n |
+| .github/workflows/output2.yml:48:14:51:48 | # VULNERABLE\ncat pr-number\necho "::set-output name=OUTPUT::SAFE"\n | semmle.label | # VULNERABLE\ncat pr-number\necho "::set-output name=OUTPUT::SAFE"\n |
+| .github/workflows/output2.yml:53:14:56:19 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\nls *.txt\n | semmle.label | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\nls *.txt\n |
 subpaths
 #select
 | .github/workflows/output1.yml:10:14:13:50 | # VULNERABLE\necho "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$BODY" >> $GITHUB_OUTPUT\n | .github/workflows/output1.yml:9:18:9:49 | github.event.comment.body | .github/workflows/output1.yml:10:14:13:50 | # VULNERABLE\necho "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$BODY" >> $GITHUB_OUTPUT\n | Potential clobbering of a step output in $@. | .github/workflows/output1.yml:10:14:13:50 | # VULNERABLE\necho "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$BODY" >> $GITHUB_OUTPUT\n | # VULNERABLE\necho "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$BODY" >> $GITHUB_OUTPUT\n |
 | .github/workflows/output1.yml:36:14:38:58 | echo "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$(<pr-number)" >> $GITHUB_OUTPUT\n | .github/workflows/output1.yml:30:9:35:6 | Uses Step | .github/workflows/output1.yml:36:14:38:58 | echo "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$(<pr-number)" >> $GITHUB_OUTPUT\n | Potential clobbering of a step output in $@. | .github/workflows/output1.yml:36:14:38:58 | echo "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$(<pr-number)" >> $GITHUB_OUTPUT\n | echo "OUTPUT_1=HARDCODED" >> $GITHUB_OUTPUT\necho "OUTPUT_2=$(<pr-number)" >> $GITHUB_OUTPUT\n |
+| .github/workflows/output2.yml:10:14:13:48 | # VULNERABLE\necho $BODY\necho "::set-output name=OUTPUT::SAFE"\n | .github/workflows/output2.yml:9:18:9:49 | github.event.comment.body | .github/workflows/output2.yml:10:14:13:48 | # VULNERABLE\necho $BODY\necho "::set-output name=OUTPUT::SAFE"\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:10:14:13:48 | # VULNERABLE\necho $BODY\necho "::set-output name=OUTPUT::SAFE"\n | # VULNERABLE\necho $BODY\necho "::set-output name=OUTPUT::SAFE"\n |
+| .github/workflows/output2.yml:17:14:20:21 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\necho $BODY\n | .github/workflows/output2.yml:16:18:16:49 | github.event.comment.body | .github/workflows/output2.yml:17:14:20:21 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\necho $BODY\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:17:14:20:21 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\necho $BODY\n | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\necho $BODY\n |
+| .github/workflows/output2.yml:42:14:46:48 | # VULNERABLE\nPR="$(<pr-number)"\necho "$PR"\necho "::set-output name=OUTPUT::SAFE"\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:42:14:46:48 | # VULNERABLE\nPR="$(<pr-number)"\necho "$PR"\necho "::set-output name=OUTPUT::SAFE"\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:42:14:46:48 | # VULNERABLE\nPR="$(<pr-number)"\necho "$PR"\necho "::set-output name=OUTPUT::SAFE"\n | # VULNERABLE\nPR="$(<pr-number)"\necho "$PR"\necho "::set-output name=OUTPUT::SAFE"\n |
+| .github/workflows/output2.yml:48:14:51:48 | # VULNERABLE\ncat pr-number\necho "::set-output name=OUTPUT::SAFE"\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:48:14:51:48 | # VULNERABLE\ncat pr-number\necho "::set-output name=OUTPUT::SAFE"\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:48:14:51:48 | # VULNERABLE\ncat pr-number\necho "::set-output name=OUTPUT::SAFE"\n | # VULNERABLE\ncat pr-number\necho "::set-output name=OUTPUT::SAFE"\n |
+| .github/workflows/output2.yml:53:14:56:19 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\nls *.txt\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:53:14:56:19 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\nls *.txt\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:53:14:56:19 | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\nls *.txt\n | # VULNERABLE\necho "::set-output name=OUTPUT::SAFE"\nls *.txt\n |