Skip to content

Commit 44724a8

Browse files
JarLobJarLob
committed
Reduce false positives in the clobbering query
1 parent 1764c65 commit 44724a8

4 files changed

Lines changed: 59 additions & 1 deletion

File tree

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,21 @@ 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+
)
121+
}
122+
123+
bindingset[command]
124+
private predicate jqProducesJsonEncodedOutput(string command) {
125+
command.regexpMatch("jq\\s+((\\.[^\\s]*)|('[^']*')|(\"[^\"]*\"))(\\s+.*)?") and
126+
not jqUsesRawOutput(command)
127+
}
128+
114129
/**
115130
* - id: clob1
116131
* run: |
@@ -165,7 +180,8 @@ class WorkflowCommandClobberingFromFileReadSink extends OutputClobberingSink {
165180
// - run: cat pr-id.txt
166181
clobbering_stmt.indexOf(clobbering_cmd) = 0
167182
)
168-
)
183+
) and
184+
not jqProducesJsonEncodedOutput(clobbering_cmd)
169185
)
170186
}
171187
}

‎actions/ql/src/change-notes/2026-07-29-output-clobbering-jq-precision.md‎

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 JSON-encoded `jq` output unless raw output is enabled.

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

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,3 +60,29 @@ 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"

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

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ 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 |
1014
nodes
1115
| .github/workflows/output1.yml:9:18:9:49 | github.event.comment.body | semmle.label | github.event.comment.body |
1216
| .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 +25,10 @@ nodes
2125
| .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 |
2226
| .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 |
2327
| .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 |
28+
| .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 |
29+
| .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 |
30+
| .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 |
31+
| .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 |
2432
subpaths
2533
#select
2634
| .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 +39,7 @@ subpaths
3139
| .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 |
3240
| .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 |
3341
| .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 |
42+
| .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 |
43+
| .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 |
44+
| .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 |
45+
| .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 |

0 commit comments

Comments
 (0)