Skip to content

Commit 3b46482

Browse files
authored
Merge pull request #22252 from JarLob/fix/actions-output-clobbering-jq-precision
Actions: Avoid output-clobbering alerts for JSON-encoded jq output
2 parents 62552ee + 5b96fcc commit 3b46482

4 files changed

Lines changed: 124 additions & 1 deletion

File tree

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

Lines changed: 39 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,43 @@ class WorkflowCommandClobberingFromEnvVarSink extends OutputClobberingSink {
111111
}
112112
}
113113

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\\}"
142+
}
143+
144+
bindingset[command]
145+
private predicate jqProducesJsonEncodedOutput(string command) {
146+
command
147+
.regexpMatch("jq(\\s+" + jqSafeOptionRegexp() + ")*\\s+" + jqSimpleFilterArgumentRegexp() +
148+
"(\\s+" + jqSafeOptionRegexp() + ")*(\\s+" + jqLiteralInputRegexp() + ")*")
149+
}
150+
114151
/**
115152
* - id: clob1
116153
* run: |
@@ -165,7 +202,8 @@ class WorkflowCommandClobberingFromFileReadSink extends OutputClobberingSink {
165202
// - run: cat pr-id.txt
166203
clobbering_stmt.indexOf(clobbering_cmd) = 0
167204
)
168-
)
205+
) and
206+
not jqProducesJsonEncodedOutput(clobbering_cmd)
169207
)
170208
}
171209
}
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
---
2+
category: minorAnalysis
3+
---
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: 54 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,57 @@ 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: clob5
64+
run: |
65+
# NOT VULNERABLE: jq emits JSON-encoded strings by default
66+
jq '.value' pr-number.json
67+
- id: clob6
68+
run: |
69+
# VULNERABLE: raw output can begin with a workflow command
70+
jq -r '.value' pr-number.json
71+
- id: clob7
72+
run: |
73+
# VULNERABLE: long raw-output option after the filter
74+
jq '.value' --raw-output pr-number.json
75+
- id: clob8
76+
run: |
77+
# VULNERABLE: combined short options include raw output
78+
jq -Mcr '.value' pr-number.json
79+
- id: clob9
80+
run: |
81+
# NOT VULNERABLE: assigned jq output remains JSON encoded
82+
VALUE=$(jq '.value' pr-number.json)
83+
echo "$VALUE"
84+
- id: clob10
85+
run: |
86+
# VULNERABLE: assigned raw output can begin with a workflow command
87+
VALUE=$(jq --raw-output '.value' pr-number.json)
88+
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: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,15 @@ 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:68:14:70:40 | # VULNERABLE: raw output can begin with a workflow command\njq -r '.value' pr-number.json\n | provenance | Config |
11+
| .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 |
12+
| .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 |
13+
| .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 |
1019
nodes
1120
| .github/workflows/output1.yml:9:18:9:49 | github.event.comment.body | semmle.label | github.event.comment.body |
1221
| .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 +30,15 @@ nodes
2130
| .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 |
2231
| .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 |
2332
| .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 |
33+
| .github/workflows/output2.yml:68:14:70:40 | # VULNERABLE: raw output can begin with a workflow command\njq -r '.value' pr-number.json\n | semmle.label | # VULNERABLE: raw output can begin with a workflow command\njq -r '.value' pr-number.json\n |
34+
| .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 |
35+
| .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 |
36+
| .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 |
2442
subpaths
2543
#select
2644
| .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 +49,12 @@ subpaths
3149
| .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 |
3250
| .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 |
3351
| .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 |
52+
| .github/workflows/output2.yml:68:14:70:40 | # VULNERABLE: raw output can begin with a workflow command\njq -r '.value' pr-number.json\n | .github/workflows/output2.yml:36:9:41:6 | Uses Step | .github/workflows/output2.yml:68:14:70:40 | # VULNERABLE: raw output can begin with a workflow command\njq -r '.value' pr-number.json\n | Potential clobbering of a step output in $@. | .github/workflows/output2.yml:68:14:70:40 | # VULNERABLE: raw output can begin with a workflow command\njq -r '.value' pr-number.json\n | # VULNERABLE: raw output can begin with a workflow command\njq -r '.value' pr-number.json\n |
53+
| .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 |
54+
| .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 |
55+
| .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)