Skip to content

Commit c745d74

Browse files
JarLobJarLob
committed
The regex wasn't escaping external input
1 parent 0751408 commit c745d74

4 files changed

Lines changed: 24 additions & 1 deletion

File tree

‎actions/ql/lib/codeql/actions/security/OutputClobberingQuery.qll‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -159,7 +159,10 @@ class WorkflowCommandClobberingFromFileReadSink extends OutputClobberingSink {
159159
clobbering_cmd.regexpMatch(["ls", Bash::fileReadCommand()] + "\\s.*") and
160160
(
161161
// - run: echo "foo=$(<pr-id.txt)"
162-
clobbering_stmt.regexpMatch("echo.*" + clobbering_cmd + ".*")
162+
exists(string echo, int echoOffset |
163+
echo = clobbering_stmt.regexpFind("\\becho\\s+", _, echoOffset) and
164+
clobbering_stmt.indexOf(clobbering_cmd, 0, echoOffset + echo.length()) >= 0
165+
)
163166
or
164167
// A file content is printed to stdout
165168
// - run: cat pr-id.txt

‎actions/ql/src/change-notes/2026-07-28-output-clobbering-regex.md‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: fix
3+
---
4+
* Fixed a performance issue in the `actions/output-clobbering/high` query caused by using unescaped source-code input in a regular expression.

‎actions/ql/test/query-tests/Security/CWE-074/.github/workflows/output2.yml‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,13 @@ jobs:
6060
CURRENT_VERSION=$(cat gradle.properties | sed -n '/^version=/ { s/^version=//;p }')
6161
echo "$CURRENT_VERSION"
6262
echo "::set-output name=OUTPUT::SAFE"
63+
- id: clob11
64+
run: |
65+
# VULNERABLE: the file name contains regex metacharacters
66+
echo "VALUE=$(cat 'pr[number](final).txt')"
67+
echo "::set-output name=OUTPUT::SAFE"
68+
- id: clob12
69+
run: |
70+
# VULNERABLE: echo is invoked through env
71+
env echo "VALUE=$(cat 'pr[number](final).txt')"
72+
echo "::set-output name=OUTPUT::SAFE"

‎actions/ql/test/query-tests/Security/CWE-074/OutputClobberingHigh.expected‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@ edges
77
| .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 | Config |
88
| .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 | Config |
99
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:58:14:62:48 | # VULNERABLE\nCURRENT_VERSION=$(cat gradle.properties \| sed -n '/^version=/ { s/^version=//;p }')\necho "$CURRENT_VERSION"\necho "::set-output name=OUTPUT::SAFE"\n | provenance | Config |
10+
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:64:14:67:48 | # VULNERABLE: the file name contains regex metacharacters\necho "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | provenance | Config |
11+
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:69:14:72:48 | # VULNERABLE: echo is invoked through env\nenv echo "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | provenance | Config |
1012
nodes
1113
| .github/workflows/output1.yml:9:18:9:49 | github.event.comment.body | semmle.label | github.event.comment.body |
1214
| .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 |
@@ -21,6 +23,8 @@ nodes
2123
| .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 |
2224
| .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 |
2325
| .github/workflows/output2.yml:58:14:62:48 | # VULNERABLE\nCURRENT_VERSION=$(cat gradle.properties \| sed -n '/^version=/ { s/^version=//;p }')\necho "$CURRENT_VERSION"\necho "::set-output name=OUTPUT::SAFE"\n | semmle.label | # VULNERABLE\nCURRENT_VERSION=$(cat gradle.properties \| sed -n '/^version=/ { s/^version=//;p }')\necho "$CURRENT_VERSION"\necho "::set-output name=OUTPUT::SAFE"\n |
26+
| .github/workflows/output2.yml:64:14:67:48 | # VULNERABLE: the file name contains regex metacharacters\necho "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | semmle.label | # VULNERABLE: the file name contains regex metacharacters\necho "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n |
27+
| .github/workflows/output2.yml:69:14:72:48 | # VULNERABLE: echo is invoked through env\nenv echo "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | semmle.label | # VULNERABLE: echo is invoked through env\nenv echo "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n |
2428
subpaths
2529
#select
2630
| .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 |
@@ -31,3 +35,5 @@ subpaths
3135
| .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 |
3236
| .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 |
3337
| .github/workflows/output2.yml:58:14:62:48 | # VULNERABLE\nCURRENT_VERSION=$(cat gradle.properties \| sed -n '/^version=/ { s/^version=//;p }')\necho "$CURRENT_VERSION"\necho "::set-output name=OUTPUT::SAFE"\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:58:14:62:48 | # VULNERABLE\nCURRENT_VERSION=$(cat gradle.properties \| sed -n '/^version=/ { s/^version=//;p }')\necho "$CURRENT_VERSION"\necho "::set-output name=OUTPUT::SAFE"\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:58:14:62:48 | # VULNERABLE\nCURRENT_VERSION=$(cat gradle.properties \| sed -n '/^version=/ { s/^version=//;p }')\necho "$CURRENT_VERSION"\necho "::set-output name=OUTPUT::SAFE"\n | # VULNERABLE\nCURRENT_VERSION=$(cat gradle.properties \| sed -n '/^version=/ { s/^version=//;p }')\necho "$CURRENT_VERSION"\necho "::set-output name=OUTPUT::SAFE"\n |
38+
| .github/workflows/output2.yml:64:14:67:48 | # VULNERABLE: the file name contains regex metacharacters\necho "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:64:14:67:48 | # VULNERABLE: the file name contains regex metacharacters\necho "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:64:14:67:48 | # VULNERABLE: the file name contains regex metacharacters\necho "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | # VULNERABLE: the file name contains regex metacharacters\necho "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n |
39+
| .github/workflows/output2.yml:69:14:72:48 | # VULNERABLE: echo is invoked through env\nenv echo "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:69:14:72:48 | # VULNERABLE: echo is invoked through env\nenv echo "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:69:14:72:48 | # VULNERABLE: echo is invoked through env\nenv echo "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n | # VULNERABLE: echo is invoked through env\nenv echo "VALUE=$(cat 'pr[number](final).txt')"\necho "::set-output name=OUTPUT::SAFE"\n |

0 commit comments

Comments
 (0)