Skip to content

Commit 5b96fcc

Browse files
committed
Restrict jq output-clobbering suppression
1 parent 44724a8 commit 5b96fcc

4 files changed

Lines changed: 75 additions & 10 deletions

File tree

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

Lines changed: 31 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -111,19 +111,41 @@ class WorkflowCommandClobberingFromEnvVarSink extends OutputClobberingSink {
111111
}
112112
}
113113

114-
bindingset[command]
115-
private predicate jqUsesRawOutput(string command) {
116-
exists(
117-
command
118-
.regexpFind("(^|\\s)(--raw-output0?|--join-output)(\\s|$)|(^|\\s)-[A-Za-z0-9]*[rj][A-Za-z0-9]*(\\s|$)",
119-
_, _)
120-
)
114+
private string jqSafeOptionRegexp() {
115+
result = "-[acCMeRnSs]+"
116+
or
117+
result =
118+
"--(ascii-output|color-output|compact-output|exit-status|monochrome-output|null-input|" +
119+
"raw-input|slurp|sort-keys|unbuffered)"
120+
}
121+
122+
private string jqSimpleFilterRegexp() {
123+
result = "\\."
124+
or
125+
result = "\\.[A-Za-z_][A-Za-z0-9_]*(\\.[A-Za-z_][A-Za-z0-9_]*|\\[[0-9]+\\])*"
126+
}
127+
128+
private string jqSimpleFilterArgumentRegexp() {
129+
result = jqSimpleFilterRegexp()
130+
or
131+
result = "'" + jqSimpleFilterRegexp() + "'"
132+
or
133+
result = "\"" + jqSimpleFilterRegexp() + "\""
134+
}
135+
136+
private string jqLiteralInputRegexp() {
137+
result = "[A-Za-z0-9_./][A-Za-z0-9_./-]*"
138+
or
139+
result = "\\$GITHUB_EVENT_PATH"
140+
or
141+
result = "\\$\\{GITHUB_EVENT_PATH\\}"
121142
}
122143

123144
bindingset[command]
124145
private predicate jqProducesJsonEncodedOutput(string command) {
125-
command.regexpMatch("jq\\s+((\\.[^\\s]*)|('[^']*')|(\"[^\"]*\"))(\\s+.*)?") and
126-
not jqUsesRawOutput(command)
146+
command
147+
.regexpMatch("jq(\\s+" + jqSafeOptionRegexp() + ")*\\s+" + jqSimpleFilterArgumentRegexp() +
148+
"(\\s+" + jqSafeOptionRegexp() + ")*(\\s+" + jqLiteralInputRegexp() + ")*")
127149
}
128150

129151
/**
Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
11
---
22
category: minorAnalysis
33
---
4-
* The `actions/output-clobbering/high` query no longer reports JSON-encoded `jq` output unless raw output is enabled.
4+
* The `actions/output-clobbering/high` query no longer reports simple `jq` path filters when their output remains JSON-encoded. Raw-output modes, complex filters, and unrecognized options remain reportable.

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

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,3 +86,31 @@ jobs:
8686
# VULNERABLE: assigned raw output can begin with a workflow command
8787
VALUE=$(jq --raw-output '.value' pr-number.json)
8888
echo "$VALUE"
89+
- id: clob11
90+
run: |
91+
# NOT VULNERABLE: combined options preserve JSON encoding
92+
jq -Mc '.value' pr-number.json
93+
- id: clob12
94+
run: |
95+
# NOT VULNERABLE: safe long options may follow a simple filter
96+
jq '.value' --compact-output pr-number.json
97+
- id: clob13
98+
run: |
99+
# VULNERABLE: join output emits strings without JSON encoding
100+
jq -j '.value' pr-number.json
101+
- id: clob14
102+
run: |
103+
# VULNERABLE: the long join-output option also emits raw strings
104+
jq '.value' --join-output pr-number.json
105+
- id: clob15
106+
run: |
107+
# VULNERABLE: raw-output0 emits strings without JSON encoding
108+
jq '.value' --raw-output0 pr-number.json
109+
- id: clob16
110+
run: |
111+
# VULNERABLE: stderr emits its input without JSON encoding
112+
jq '.value | stderr' pr-number.json
113+
- id: clob17
114+
run: |
115+
# VULNERABLE: halt_error emits its input without JSON encoding
116+
jq '.value | halt_error(1)' pr-number.json

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

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,11 @@ edges
1111
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:72:14:74:50 | # VULNERABLE: long raw-output option after the filter\njq '.value' --raw-output pr-number.json\n | provenance | Config |
1212
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:76:14:78:42 | # VULNERABLE: combined short options include raw output\njq -Mcr '.value' pr-number.json\n | provenance | Config |
1313
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:85:14:88:24 | # VULNERABLE: assigned raw output can begin with a workflow command\nVALUE=$(jq --raw-output '.value' pr-number.json)\necho "$VALUE"\n | provenance | Config |
14+
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:98:14:100:40 | # VULNERABLE: join output emits strings without JSON encoding\njq -j '.value' pr-number.json\n | provenance | Config |
15+
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:102:14:104:51 | # VULNERABLE: the long join-output option also emits raw strings\njq '.value' --join-output pr-number.json\n | provenance | Config |
16+
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:106:14:108:51 | # VULNERABLE: raw-output0 emits strings without JSON encoding\njq '.value' --raw-output0 pr-number.json\n | provenance | Config |
17+
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:110:14:112:46 | # VULNERABLE: stderr emits its input without JSON encoding\njq '.value \| stderr' pr-number.json\n | provenance | Config |
18+
| .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:114:14:116:53 | # VULNERABLE: halt_error emits its input without JSON encoding\njq '.value \| halt_error(1)' pr-number.json\n | provenance | Config |
1419
nodes
1520
| .github/workflows/output1.yml:9:18:9:49 | github.event.comment.body | semmle.label | github.event.comment.body |
1621
| .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 |
@@ -29,6 +34,11 @@ nodes
2934
| .github/workflows/output2.yml:72:14:74:50 | # VULNERABLE: long raw-output option after the filter\njq '.value' --raw-output pr-number.json\n | semmle.label | # VULNERABLE: long raw-output option after the filter\njq '.value' --raw-output pr-number.json\n |
3035
| .github/workflows/output2.yml:76:14:78:42 | # VULNERABLE: combined short options include raw output\njq -Mcr '.value' pr-number.json\n | semmle.label | # VULNERABLE: combined short options include raw output\njq -Mcr '.value' pr-number.json\n |
3136
| .github/workflows/output2.yml:85:14:88:24 | # VULNERABLE: assigned raw output can begin with a workflow command\nVALUE=$(jq --raw-output '.value' pr-number.json)\necho "$VALUE"\n | semmle.label | # VULNERABLE: assigned raw output can begin with a workflow command\nVALUE=$(jq --raw-output '.value' pr-number.json)\necho "$VALUE"\n |
37+
| .github/workflows/output2.yml:98:14:100:40 | # VULNERABLE: join output emits strings without JSON encoding\njq -j '.value' pr-number.json\n | semmle.label | # VULNERABLE: join output emits strings without JSON encoding\njq -j '.value' pr-number.json\n |
38+
| .github/workflows/output2.yml:102:14:104:51 | # VULNERABLE: the long join-output option also emits raw strings\njq '.value' --join-output pr-number.json\n | semmle.label | # VULNERABLE: the long join-output option also emits raw strings\njq '.value' --join-output pr-number.json\n |
39+
| .github/workflows/output2.yml:106:14:108:51 | # VULNERABLE: raw-output0 emits strings without JSON encoding\njq '.value' --raw-output0 pr-number.json\n | semmle.label | # VULNERABLE: raw-output0 emits strings without JSON encoding\njq '.value' --raw-output0 pr-number.json\n |
40+
| .github/workflows/output2.yml:110:14:112:46 | # VULNERABLE: stderr emits its input without JSON encoding\njq '.value \| stderr' pr-number.json\n | semmle.label | # VULNERABLE: stderr emits its input without JSON encoding\njq '.value \| stderr' pr-number.json\n |
41+
| .github/workflows/output2.yml:114:14:116:53 | # VULNERABLE: halt_error emits its input without JSON encoding\njq '.value \| halt_error(1)' pr-number.json\n | semmle.label | # VULNERABLE: halt_error emits its input without JSON encoding\njq '.value \| halt_error(1)' pr-number.json\n |
3242
subpaths
3343
#select
3444
| .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 |
@@ -43,3 +53,8 @@ subpaths
4353
| .github/workflows/output2.yml:72:14:74:50 | # VULNERABLE: long raw-output option after the filter\njq '.value' --raw-output pr-number.json\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:72:14:74:50 | # VULNERABLE: long raw-output option after the filter\njq '.value' --raw-output pr-number.json\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:72:14:74:50 | # VULNERABLE: long raw-output option after the filter\njq '.value' --raw-output pr-number.json\n | # VULNERABLE: long raw-output option after the filter\njq '.value' --raw-output pr-number.json\n |
4454
| .github/workflows/output2.yml:76:14:78:42 | # VULNERABLE: combined short options include raw output\njq -Mcr '.value' pr-number.json\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:76:14:78:42 | # VULNERABLE: combined short options include raw output\njq -Mcr '.value' pr-number.json\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:76:14:78:42 | # VULNERABLE: combined short options include raw output\njq -Mcr '.value' pr-number.json\n | # VULNERABLE: combined short options include raw output\njq -Mcr '.value' pr-number.json\n |
4555
| .github/workflows/output2.yml:85:14:88:24 | # VULNERABLE: assigned raw output can begin with a workflow command\nVALUE=$(jq --raw-output '.value' pr-number.json)\necho "$VALUE"\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:85:14:88:24 | # VULNERABLE: assigned raw output can begin with a workflow command\nVALUE=$(jq --raw-output '.value' pr-number.json)\necho "$VALUE"\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:85:14:88:24 | # VULNERABLE: assigned raw output can begin with a workflow command\nVALUE=$(jq --raw-output '.value' pr-number.json)\necho "$VALUE"\n | # VULNERABLE: assigned raw output can begin with a workflow command\nVALUE=$(jq --raw-output '.value' pr-number.json)\necho "$VALUE"\n |
56+
| .github/workflows/output2.yml:98:14:100:40 | # VULNERABLE: join output emits strings without JSON encoding\njq -j '.value' pr-number.json\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:98:14:100:40 | # VULNERABLE: join output emits strings without JSON encoding\njq -j '.value' pr-number.json\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:98:14:100:40 | # VULNERABLE: join output emits strings without JSON encoding\njq -j '.value' pr-number.json\n | # VULNERABLE: join output emits strings without JSON encoding\njq -j '.value' pr-number.json\n |
57+
| .github/workflows/output2.yml:102:14:104:51 | # VULNERABLE: the long join-output option also emits raw strings\njq '.value' --join-output pr-number.json\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:102:14:104:51 | # VULNERABLE: the long join-output option also emits raw strings\njq '.value' --join-output pr-number.json\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:102:14:104:51 | # VULNERABLE: the long join-output option also emits raw strings\njq '.value' --join-output pr-number.json\n | # VULNERABLE: the long join-output option also emits raw strings\njq '.value' --join-output pr-number.json\n |
58+
| .github/workflows/output2.yml:106:14:108:51 | # VULNERABLE: raw-output0 emits strings without JSON encoding\njq '.value' --raw-output0 pr-number.json\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:106:14:108:51 | # VULNERABLE: raw-output0 emits strings without JSON encoding\njq '.value' --raw-output0 pr-number.json\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:106:14:108:51 | # VULNERABLE: raw-output0 emits strings without JSON encoding\njq '.value' --raw-output0 pr-number.json\n | # VULNERABLE: raw-output0 emits strings without JSON encoding\njq '.value' --raw-output0 pr-number.json\n |
59+
| .github/workflows/output2.yml:110:14:112:46 | # VULNERABLE: stderr emits its input without JSON encoding\njq '.value \| stderr' pr-number.json\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:110:14:112:46 | # VULNERABLE: stderr emits its input without JSON encoding\njq '.value \| stderr' pr-number.json\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:110:14:112:46 | # VULNERABLE: stderr emits its input without JSON encoding\njq '.value \| stderr' pr-number.json\n | # VULNERABLE: stderr emits its input without JSON encoding\njq '.value \| stderr' pr-number.json\n |
60+
| .github/workflows/output2.yml:114:14:116:53 | # VULNERABLE: halt_error emits its input without JSON encoding\njq '.value \| halt_error(1)' pr-number.json\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:114:14:116:53 | # VULNERABLE: halt_error emits its input without JSON encoding\njq '.value \| halt_error(1)' pr-number.json\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:114:14:116:53 | # VULNERABLE: halt_error emits its input without JSON encoding\njq '.value \| halt_error(1)' pr-number.json\n | # VULNERABLE: halt_error emits its input without JSON encoding\njq '.value \| halt_error(1)' pr-number.json\n |

0 commit comments

Comments
 (0)