Skip to content

Commit eb4eb4e

Browse files
author
Alvaro Muñoz
authored
Merge branch 'master' into cache_poisoning_actions
2 parents d6fb0ae + c39e802 commit eb4eb4e

File tree

14 files changed

+334
-94
lines changed

14 files changed

+334
-94
lines changed

‎.github/workflows/copy-to-bughalla.yml

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,9 @@
11
name: Copy to Bughalla
22

3-
on: push
3+
on:
4+
push:
5+
branches:
6+
- 'master'
47

58
permissions:
69
contents: read

‎ql/lib/codeql/actions/Ast.qll

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,6 +375,8 @@ abstract class Uses extends AstNode instanceof UsesImpl {
375375

376376
string getVersion() { result = super.getVersion() }
377377

378+
int getMajorVersion() { result = super.getMajorVersion() }
379+
378380
string getArgument(string argName) { result = super.getArgument(argName) }
379381

380382
Expression getArgumentExpr(string argName) { result = super.getArgumentExpr(argName) }

‎ql/lib/codeql/actions/ast/internal/Ast.qll

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -839,6 +839,8 @@ abstract class UsesImpl extends AstNodeImpl {
839839

840840
abstract string getVersion();
841841

842+
int getMajorVersion() { result = this.getVersion().regexpReplaceAll("\\..*", "").toInt() }
843+
842844
/** Gets the argument expression for the given key. */
843845
string getArgument(string key) {
844846
exists(ScalarValueImpl scalar |
@@ -883,8 +885,10 @@ class UsesStepImpl extends StepImpl, UsesImpl {
883885
).toLowerCase()
884886
}
885887

886-
/** Gets the version reference used when checking out the Action, e.g. `v2` in `actions/checkout@v2`. */
887-
override string getVersion() { result = u.getValue().regexpCapture(usesParser(), 3) }
888+
/** Gets the version reference used when checking out the Action, e.g. `2` in `actions/checkout@v2`. */
889+
override string getVersion() {
890+
result = u.getValue().regexpCapture(usesParser(), 3).regexpReplaceAll("^v", "")
891+
}
888892

889893
override string toString() {
890894
if exists(this.getId()) then result = "Uses Step: " + this.getId() else result = "Uses Step"
@@ -916,12 +920,12 @@ class ExternalJobImpl extends JobImpl, UsesImpl {
916920
u.getValue().regexpCapture(repoUsesParser(), 3)
917921
}
918922

919-
/** Gets the version reference used when checking out the Action, e.g. `v2` in `actions/checkout@v2`. */
923+
/** Gets the version reference used when checking out the Action, e.g. `2` in `actions/checkout@v2`. */
920924
override string getVersion() {
921925
exists(YamlString name |
922926
n.lookup("uses") = name and
923927
if not name.getValue().matches("\\.%")
924-
then result = name.getValue().regexpCapture(repoUsesParser(), 4)
928+
then result = name.getValue().regexpCapture(repoUsesParser(), 4).regexpReplaceAll("^v", "")
925929
else none()
926930
)
927931
}

‎ql/lib/codeql/actions/dataflow/FlowSources.qll

Lines changed: 112 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,10 +252,121 @@ class CompositeActionInputSource extends RemoteFlowSource {
252252
}
253253

254254
/**
255-
* A downloadeded artifact.
255+
* A downloaded artifact.
256256
*/
257257
private class ArtifactSource extends RemoteFlowSource {
258258
ArtifactSource() { this.asExpr() instanceof UntrustedArtifactDownloadStep }
259259

260260
override string getSourceType() { result = "artifact" }
261261
}
262+
263+
/**
264+
* A list of file names returned by dorny/paths-filter.
265+
*/
266+
class DornyPathsFilterSource extends RemoteFlowSource {
267+
DornyPathsFilterSource() {
268+
exists(UsesStep u |
269+
u.getCallee() = "dorny/paths-filter" and
270+
u.getArgument("list-files") = ["csv", "json"] and
271+
this.asExpr() = u
272+
)
273+
}
274+
275+
override string getSourceType() { result = "filename" }
276+
}
277+
278+
/**
279+
* A list of file names returned by tj-actions/changed-files.
280+
*/
281+
class TJActionsChangedFilesSource extends RemoteFlowSource {
282+
TJActionsChangedFilesSource() {
283+
exists(UsesStep u |
284+
u.getCallee() = "tj-actions/changed-files" and
285+
(
286+
u.getArgument("safe_output") = "false" or
287+
u.getMajorVersion() < 41 or
288+
u.getVersion()
289+
.matches([
290+
"56284d8", "9454999", "1c93849", "da093c1", "25ef392", "18c8a4e", "4052680",
291+
"bfc49f4", "af292f1", "56284d8", "fea790c", "95690f9", "408093d", "db153ba",
292+
"8238a41", "4196030", "a21a533", "8e79ba7", "76c4d81", "6ee9cdc", "246636f",
293+
"48566bb", "fea790c", "1aee362", "2f7246c", "0fc9663", "c860b5c", "2f8b802",
294+
"b7f1b73", "1c26215", "17f3fec", "1aee362", "a0585ff", "87697c0", "85c8b82",
295+
"a96679d", "920e7b9", "de0eba3", "3928317", "68b429d", "2a968ff", "1f20fb8",
296+
"87e23c4", "54849de", "bb33761", "ec1e14c", "2106eb4", "e5efec4", "5817a9e",
297+
"a0585ff", "54479c3", "e1754a4", "9bf0914", "c912451", "174a2a6", "fb20f4d",
298+
"07e0177", "b137868", "1aae160", "5d2fcdb", "9ecc6e7", "8c9ee56", "5978e5a",
299+
"17c3e9e", "3f7b5c9", "cf4fe87", "043929e", "4e2535f", "652648a", "9ad1a5b",
300+
"c798a4e", "25eaddf", "abef388", "1c2673b", "53c377a", "54479c3", "039afcd",
301+
"b2d17f5", "4a0aac0", "ce810b2", "7ecfc67", "b109d83", "79adacd", "6e426e6",
302+
"5e2d64b", "e9b5807", "db5dd7c", "07f86bc", "3a3ec49", "ee13744", "cda2902",
303+
"9328bab", "4e680e1", "bd376fb", "84ed30e", "74b06ca", "5ce975c", "04124ef",
304+
"3ee6abf", "23e3c43", "5a331a4", "7433886", "d5414fd", "7f2aa19", "210cc83",
305+
"db3ea27", "57d9664", "0953088", "0562b9f", "487675b", "9a6dabf", "7839ede",
306+
"c2296c1", "ea251d4", "1d1287f", "392359f", "7f33882", "1d8a2f9", "0626c3f",
307+
"a2b1e5d", "110b9ba", "039afcd", "ce4b8e3", "3b6c057", "4f64429", "3f1e44a",
308+
"74dc2e8", "8356a01", "baaf598", "8a4cc4f", "8a7336f", "3996bc3", "ef0a290",
309+
"3ebdc42", "94e6fba", "3dbb79f", "991e8b3", "72d3bb8", "72d3bb8", "5f89dc7",
310+
"734bb16", "d2e030b", "6ba3c59", "d0e4477", "b91acef", "1263363", "7184077",
311+
"cbfb0fd", "932dad3", "9f28968", "c4d29bf", "ce4b8e3", "aa52cfc", "aa52cfc",
312+
"1d6e210", "8953e85", "8de562e", "7c640bd", "2706452", "1d6e210", "dd7c814",
313+
"528984a", "75af1a4", "5184a75", "dd7c814", "402f382", "402f382", "f7a5640",
314+
"df4daca", "602081b", "6e12407", "c5c9b6f", "c41b715", "60f4aab", "82edb42",
315+
"18edda7", "bec82eb", "f7a5640", "28ac672", "602cf94", "5e56dca", "58ae566",
316+
"7394701", "36e65a1", "bf6ddb7", "6c44eb8", "b2ee165", "34a865a", "fb1fe28",
317+
"ae90a0b", "bc1dc8f", "3de1f9a", "0edfedf", "2054502", "944a8b8", "581eef0",
318+
"e55f7fb", "07b38ce", "d262520", "a6d456f", "a59f800", "a2f1692", "72aab29",
319+
"e35d0af", "081ee9c", "1f30bd2", "227e314", "ffd30e8", "f5a8de7", "0bc7d40",
320+
"a53d74f", "9335416", "4daffba", "4b1f26a", "09441d3", "e44053b", "c0dba81",
321+
"fd2e991", "2a8a501", "a8ea720", "88edda5", "be68c10", "b59431b", "68bd279",
322+
"2c85495", "f276697", "00f80ef", "f56e736", "019a09d", "3b638a9", "b42f932",
323+
"8dfe0ee", "aae164d", "09a8797", "b54a7ae", "902e607", "2b51570", "040111b",
324+
"3b638a9", "1d34e69", "b86b537", "2a771ad", "75933dc", "2c0d12b", "7abdbc9",
325+
"675ab58", "8c6f276", "d825b1f", "0bd70b7", "0fe67a1", "7bfa539", "d679de9",
326+
"1e10ed4", "0754fda", "d290bdd", "15b1769", "2ecd06d", "5fe8e4d", "7c66aa2",
327+
"2ecd06d", "e95bba8", "7852058", "81f32e2", "450eadf", "0e956bb", "300e935",
328+
"fcb2ab8", "271bbd6", "e8ace01", "473984b", "032f37f", "3a35bdf", "c2216f6",
329+
"0f16c26", "271468e", "fb063fc", "a05436f", "c061ef1", "489e2d5", "8d5a33c",
330+
"fbfaba5", "1980f55", "a86b560", "f917cc3", "e18ccae", "e1d275d", "00f80ef",
331+
"9c1a181", "5eaa2d8", "188487d", "3098891", "467d26c", "d9eb683", "09a8797",
332+
"8e7cc77", "81ad4b8", "5e2a2f1", "1af9ab3", "55a857d", "62a9200", "b915d09",
333+
"f0751de", "eef9423"
334+
] + "%")
335+
) and
336+
this.asExpr() = u
337+
)
338+
}
339+
340+
override string getSourceType() { result = "filename" }
341+
}
342+
343+
/**
344+
* A list of file names returned by tj-actions/verify-changed-files.
345+
*/
346+
class TJActionsVerifyChangedFilesSource extends RemoteFlowSource {
347+
TJActionsVerifyChangedFilesSource() {
348+
exists(UsesStep u |
349+
u.getCallee() = "tj-actions/verify-changed-files" and
350+
(
351+
u.getArgument("safe_output") = "false" or
352+
u.getMajorVersion() < 17 or
353+
u.getVersion()
354+
.matches([
355+
"54e20d3", "a9b6fd3", "30aa174", "7f1b21c", "54e20d3", "0409e18", "7da22d0",
356+
"7016858", "0409e18", "7517b83", "bad2f5d", "3b573ac", "7517b83", "f557547",
357+
"9ed3155", "f557547", "a3391b5", "a3391b5", "1d7ee97", "c432297", "6e986df",
358+
"fa6ea30", "6f40ee1", "1b13d25", "c09bcad", "fda469d", "bd1e271", "367ba21",
359+
"9dea97e", "c154cc6", "527ff75", "e8756d5", "bcb4e76", "25267f5", "ea24bfd",
360+
"f2a40ba", "197e121", "a8f1b11", "95c26dd", "97ba4cc", "68310bb", "720ba6a",
361+
"cedd709", "d68d3d2", "2e1153b", "c3dd635", "81bd1de", "31a9c74", "e981d37",
362+
"e7f801c", "e86d0b9", "ad255a4", "3a8aed1", "de910b5", "d31b2a1", "e61c6fc",
363+
"380890d", "873cfd6", "b0c60c8", "7183183", "6555389", "9828a95", "8150cee",
364+
"48ddf88"
365+
] + "%")
366+
) and
367+
this.asExpr() = u
368+
)
369+
}
370+
371+
override string getSourceType() { result = "filename" }
372+
}

‎ql/lib/codeql/actions/dataflow/FlowSteps.qll

Lines changed: 52 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
private import actions
66
private import codeql.util.Unit
77
private import codeql.actions.DataFlow
8+
private import codeql.actions.dataflow.FlowSources
89
private import codeql.actions.dataflow.ExternalFlow
910
private import codeql.actions.security.ArtifactPoisoningQuery
1011

@@ -43,10 +44,6 @@ predicate envToRunStep(DataFlow::Node pred, DataFlow::Node succ) {
4344
)
4445
}
4546

46-
class EnvToRunTaintStep extends AdditionalTaintStep {
47-
override predicate step(DataFlow::Node node1, DataFlow::Node node2) { envToRunStep(node1, node2) }
48-
}
49-
5047
/**
5148
* Holds if a Run step declares an environment variable, uses it in its script and sets an output in its script.
5249
* e.g.
@@ -119,8 +116,57 @@ predicate artifactDownloadToUseStep(DataFlow::Node pred, DataFlow::Node succ) {
119116
)
120117
}
121118

122-
class ArtifactDownloadToUseTaintStep extends AdditionalTaintStep {
119+
/**
120+
* A read of the _files field of the dorny/paths-filter action.
121+
*/
122+
predicate dornyPathsFilterTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
123+
exists(StepsExpression o |
124+
pred instanceof DornyPathsFilterSource and
125+
o.getStepId() = pred.asExpr().(UsesStep).getId() and
126+
o.getFieldName().matches("%_files") and
127+
succ.asExpr() = o
128+
)
129+
}
130+
131+
/**
132+
* A read of user-controlled field of the tj-actions/changed-files action.
133+
*/
134+
predicate tjActionsChangedFilesTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
135+
exists(StepsExpression o |
136+
pred instanceof TJActionsChangedFilesSource and
137+
o.getTarget() = pred.asExpr() and
138+
o.getStepId() = pred.asExpr().(UsesStep).getId() and
139+
o.getFieldName() =
140+
[
141+
"added_files", "copied_files", "deleted_files", "modified_files", "renamed_files",
142+
"all_old_new_renamed_files", "type_changed_files", "unmerged_files", "unknown_files",
143+
"all_changed_and_modified_files", "all_changed_files", "other_changed_files",
144+
"all_modified_files", "other_modified_files", "other_deleted_files", "modified_keys",
145+
"changed_keys"
146+
] and
147+
succ.asExpr() = o
148+
)
149+
}
150+
151+
/**
152+
* A read of user-controlled field of the tj-actions/verify-changed-files action.
153+
*/
154+
predicate tjActionsVerifyChangedFilesTaintStep(DataFlow::Node pred, DataFlow::Node succ) {
155+
exists(StepsExpression o |
156+
pred instanceof TJActionsChangedFilesSource and
157+
o.getTarget() = pred.asExpr() and
158+
o.getStepId() = pred.asExpr().(UsesStep).getId() and
159+
o.getFieldName() = "changed_files" and
160+
succ.asExpr() = o
161+
)
162+
}
163+
164+
class TaintSteps extends AdditionalTaintStep {
123165
override predicate step(DataFlow::Node node1, DataFlow::Node node2) {
124-
artifactDownloadToUseStep(node1, node2)
166+
envToRunStep(node1, node2) or
167+
artifactDownloadToUseStep(node1, node2) or
168+
dornyPathsFilterTaintStep(node1, node2) or
169+
tjActionsChangedFilesTaintStep(node1, node2) or
170+
tjActionsVerifyChangedFilesTaintStep(node1, node2)
125171
}
126172
}

‎ql/lib/ext/dorny_paths-filter.model.yml

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎ql/lib/ext/tj-actions_changed-files.model.yml

Lines changed: 0 additions & 22 deletions
This file was deleted.

‎ql/lib/ext/tj-actions_verify-changed-files.model.yml

Lines changed: 0 additions & 6 deletions
This file was deleted.

‎ql/test/library-tests/test.expected

Lines changed: 0 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -438,7 +438,6 @@ sources
438438
| amannn/action-semantic-pull-request | * | output.error_message | text | manual |
439439
| cypress-io/github-action | * | env.GH_BRANCH | branch | manual |
440440
| dawidd6/action-download-artifact | * | output.artifacts | artifact | manual |
441-
| dorny/paths-filter | * | output.changes | filename | manual |
442441
| franzdiebold/github-env-vars-action | * | output.CI_PR_DESCRIPTION | text | manual |
443442
| franzdiebold/github-env-vars-action | * | output.CI_PR_TITLE | title | manual |
444443
| googlecloudplatform/magic-modules | * | output.changed-files | filename | manual |
@@ -456,24 +455,6 @@ sources
456455
| tj-actions/branch-names | * | output.current_branch | branch | manual |
457456
| tj-actions/branch-names | * | output.head_ref_branch | branch | manual |
458457
| tj-actions/branch-names | * | output.ref_branch | branch | manual |
459-
| tj-actions/changed-files | * | output.added_files | filename | manual |
460-
| tj-actions/changed-files | * | output.all_changed_and_modified_files | filename | manual |
461-
| tj-actions/changed-files | * | output.all_changed_files | filename | manual |
462-
| tj-actions/changed-files | * | output.all_modified_files | filename | manual |
463-
| tj-actions/changed-files | * | output.all_old_new_renamed_files | filename | manual |
464-
| tj-actions/changed-files | * | output.changed_keys | filename | manual |
465-
| tj-actions/changed-files | * | output.copied_files | filename | manual |
466-
| tj-actions/changed-files | * | output.deleted_files | filename | manual |
467-
| tj-actions/changed-files | * | output.modified_files | filename | manual |
468-
| tj-actions/changed-files | * | output.modified_keys | filename | manual |
469-
| tj-actions/changed-files | * | output.other_changed_files | filename | manual |
470-
| tj-actions/changed-files | * | output.other_deleted_files | filename | manual |
471-
| tj-actions/changed-files | * | output.other_modified_files | filename | manual |
472-
| tj-actions/changed-files | * | output.renamed_files | filename | manual |
473-
| tj-actions/changed-files | * | output.type_changed_files | filename | manual |
474-
| tj-actions/changed-files | * | output.unknown_files | filename | manual |
475-
| tj-actions/changed-files | * | output.unmerged_files | filename | manual |
476-
| tj-actions/verify-changed-files | * | output.changed-files | filename | manual |
477458
| trilom/file-changes-action | * | output.files | filename | manual |
478459
| trilom/file-changes-action | * | output.files_added | filename | manual |
479460
| trilom/file-changes-action | * | output.files_modified | filename | manual |

‎ql/test/query-tests/Security/CWE-094/.github/workflows/changed-files.yml

Lines changed: 41 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,6 @@ name: CI
22

33
on:
44
pull_request:
5-
branches:
6-
- main
75

86
jobs:
97
changed_files:
@@ -13,13 +11,50 @@ jobs:
1311
- uses: actions/checkout@v4
1412
with:
1513
fetch-depth: 0
16-
- name: Get changed files
17-
id: changed-files
14+
15+
- name: Get changed files 1
16+
id: changed-files1
1817
uses: tj-actions/changed-files@v40
18+
- name: List all changed files 1
19+
run: |
20+
for file in ${{ steps.changed-files1.outputs.all_changed_files }}; do
21+
echo "$file was changed"
22+
done
23+
24+
- name: Get changed files 2
25+
id: changed-files2
26+
uses: tj-actions/changed-files@v41
27+
- name: List all changed files 2
28+
run: |
29+
for file in ${{ steps.changed-files2.outputs.all_changed_files }}; do
30+
echo "$file was changed"
31+
done
1932
20-
- name: List all changed files
33+
- name: Get changed files 3
34+
id: changed-files3
35+
uses: tj-actions/changed-files@v41
36+
with:
37+
safe_output: false
38+
- name: List all changed files 3
39+
run: |
40+
for file in ${{ steps.changed-files3.outputs.all_changed_files }}; do
41+
echo "$file was changed"
42+
done
43+
44+
- name: Get changed files 4
45+
id: changed-files4
46+
uses: tj-actions/changed-files@0874344d6ebbaa00a27da73276ae7162fadcaf69 # v44.3.0
47+
- name: List all changed files 4
2148
run: |
22-
for file in ${{ steps.changed-files.outputs.all_changed_files }}; do
49+
for file in ${{ steps.changed-files4.outputs.all_changed_files }}; do
2350
echo "$file was changed"
2451
done
2552
53+
- name: Get changed files 5
54+
id: changed-files5
55+
uses: tj-actions/changed-files@95690f9ece77c1740f4a55b7f1de9023ed6b1f87 # v39.2.3
56+
- name: List all changed files 5
57+
run: |
58+
for file in ${{ steps.changed-files5.outputs.all_changed_files }}; do
59+
echo "$file was changed"
60+
done

0 commit comments

Comments
 (0)