Skip to content

Commit 6a87192

Browse files
author
Alvaro Muñoz
committed
Account for insecure action versions
1 parent de74b88 commit 6a87192

File tree

6 files changed

+98
-22
lines changed

6 files changed

+98
-22
lines changed

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

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

792792
abstract string getVersion();
793793

794+
int getMajorVersion() { result = this.getVersion().regexpReplaceAll("\\..*", "").toInt() }
795+
794796
/** Gets the argument expression for the given key. */
795797
string getArgument(string key) {
796798
exists(ScalarValueImpl scalar |
@@ -832,8 +834,10 @@ class UsesStepImpl extends StepImpl, UsesImpl {
832834
).toLowerCase()
833835
}
834836

835-
/** Gets the version reference used when checking out the Action, e.g. `v2` in `actions/checkout@v2`. */
836-
override string getVersion() { result = u.getValue().regexpCapture(usesParser(), 3) }
837+
/** Gets the version reference used when checking out the Action, e.g. `2` in `actions/checkout@v2`. */
838+
override string getVersion() {
839+
result = u.getValue().regexpCapture(usesParser(), 3).regexpReplaceAll("^v", "")
840+
}
837841

838842
override string toString() {
839843
if exists(this.getId()) then result = "Uses Step: " + this.getId() else result = "Uses Step"
@@ -865,12 +869,12 @@ class ExternalJobImpl extends JobImpl, UsesImpl {
865869
u.getValue().regexpCapture(repoUsesParser(), 3)
866870
}
867871

868-
/** Gets the version reference used when checking out the Action, e.g. `v2` in `actions/checkout@v2`. */
872+
/** Gets the version reference used when checking out the Action, e.g. `2` in `actions/checkout@v2`. */
869873
override string getVersion() {
870874
exists(YamlString name |
871875
n.lookup("uses") = name and
872876
if not name.getValue().matches("\\.%")
873-
then result = name.getValue().regexpCapture(repoUsesParser(), 4)
877+
then result = name.getValue().regexpCapture(repoUsesParser(), 4).regexpReplaceAll("^v", "")
874878
else none()
875879
)
876880
}

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

Lines changed: 49 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -284,7 +284,54 @@ class TJActionsChangedFilesSource extends RemoteFlowSource {
284284
u.getCallee() = "tj-actions/changed-files" and
285285
(
286286
u.getArgument("safe_output") = "false" or
287-
u.getVersion().regexpReplaceAll("^v", "").regexpReplaceAll("\\..*", "").toInt() < 41
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+
] + "%")
288335
) and
289336
this.asExpr() = u
290337
)
@@ -302,7 +349,7 @@ class TJActionsVerifyChangedFilesSource extends RemoteFlowSource {
302349
u.getCallee() = "tj-actions/verify-changed-files" and
303350
(
304351
u.getArgument("safe_output") = "false" or
305-
u.getVersion().regexpReplaceAll("^v", "").regexpReplaceAll("\\..*", "").toInt() < 17
352+
u.getMajorVersion() < 17
306353
) and
307354
this.asExpr() = u
308355
)

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

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,3 +40,21 @@ jobs:
4040
for file in ${{ steps.changed-files3.outputs.all_changed_files }}; do
4141
echo "$file was changed"
4242
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
48+
run: |
49+
for file in ${{ steps.changed-files4.outputs.all_changed_files }}; do
50+
echo "$file was changed"
51+
done
52+
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

‎ql/test/query-tests/Security/CWE-094/CodeInjection.expected

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ edges
88
| .github/workflows/artifactpoisoning2.yml:13:9:19:6 | Uses Step: pr | .github/workflows/artifactpoisoning2.yml:22:17:22:42 | steps.pr.outputs.id |
99
| .github/workflows/changed-files.yml:15:9:18:6 | Uses Step: changed-files1 | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files |
1010
| .github/workflows/changed-files.yml:33:9:38:6 | Uses Step: changed-files3 | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files |
11+
| .github/workflows/changed-files.yml:53:9:56:6 | Uses Step: changed-files5 | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files |
1112
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log |
1213
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log |
1314
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced |
@@ -87,6 +88,8 @@ nodes
8788
| .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | semmle.label | steps.changed-files1.outputs.all_changed_files |
8889
| .github/workflows/changed-files.yml:33:9:38:6 | Uses Step: changed-files3 | semmle.label | Uses Step: changed-files3 |
8990
| .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | semmle.label | steps.changed-files3.outputs.all_changed_files |
91+
| .github/workflows/changed-files.yml:53:9:56:6 | Uses Step: changed-files5 | semmle.label | Uses Step: changed-files5 |
92+
| .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | semmle.label | steps.changed-files5.outputs.all_changed_files |
9093
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
9194
| .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log |
9295
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
@@ -247,6 +250,7 @@ subpaths
247250
#select
248251
| .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | .github/workflows/changed-files.yml:15:9:18:6 | Uses Step: changed-files1 | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | ${{ steps.changed-files1.outputs.all_changed_files }} |
249252
| .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | .github/workflows/changed-files.yml:33:9:38:6 | Uses Step: changed-files3 | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | ${{ steps.changed-files3.outputs.all_changed_files }} |
253+
| .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | .github/workflows/changed-files.yml:53:9:56:6 | Uses Step: changed-files5 | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | ${{ steps.changed-files5.outputs.all_changed_files }} |
250254
| .github/workflows/changelog.yml:58:26:58:39 | env.log | .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog.yml:58:26:58:39 | env.log | ${{ env.log }} |
251255
| .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | Potential code injection in $@, which may be controlled by an external user. | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log | ${{ env.log }} |
252256
| action1/action.yml:14:19:14:50 | github.event.comment.body | action1/action.yml:14:19:14:50 | github.event.comment.body | action1/action.yml:14:19:14:50 | github.event.comment.body | Potential code injection in $@, which may be controlled by an external user. | action1/action.yml:14:19:14:50 | github.event.comment.body | ${{ github.event.comment.body }} |

‎ql/test/query-tests/Security/CWE-094/PrivilegedCodeInjection.expected

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ edges
88
| .github/workflows/artifactpoisoning2.yml:13:9:19:6 | Uses Step: pr | .github/workflows/artifactpoisoning2.yml:22:17:22:42 | steps.pr.outputs.id |
99
| .github/workflows/changed-files.yml:15:9:18:6 | Uses Step: changed-files1 | .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files |
1010
| .github/workflows/changed-files.yml:33:9:38:6 | Uses Step: changed-files3 | .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files |
11+
| .github/workflows/changed-files.yml:53:9:56:6 | Uses Step: changed-files5 | .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files |
1112
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog.yml:58:26:58:39 | env.log |
1213
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | .github/workflows/changelog_from_prt.yml:58:26:58:39 | env.log |
1314
| .github/workflows/cross3.yml:27:7:37:4 | Uses Step: remove_quotations [replaced] | .github/workflows/cross3.yml:39:31:39:75 | steps.remove_quotations.outputs.replaced |
@@ -87,6 +88,8 @@ nodes
8788
| .github/workflows/changed-files.yml:20:24:20:76 | steps.changed-files1.outputs.all_changed_files | semmle.label | steps.changed-files1.outputs.all_changed_files |
8889
| .github/workflows/changed-files.yml:33:9:38:6 | Uses Step: changed-files3 | semmle.label | Uses Step: changed-files3 |
8990
| .github/workflows/changed-files.yml:40:24:40:76 | steps.changed-files3.outputs.all_changed_files | semmle.label | steps.changed-files3.outputs.all_changed_files |
91+
| .github/workflows/changed-files.yml:53:9:56:6 | Uses Step: changed-files5 | semmle.label | Uses Step: changed-files5 |
92+
| .github/workflows/changed-files.yml:58:24:58:76 | steps.changed-files5.outputs.all_changed_files | semmle.label | steps.changed-files5.outputs.all_changed_files |
9093
| .github/workflows/changelog.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |
9194
| .github/workflows/changelog.yml:58:26:58:39 | env.log | semmle.label | env.log |
9295
| .github/workflows/changelog_from_prt.yml:49:19:49:56 | github.event.pull_request.title | semmle.label | github.event.pull_request.title |

0 commit comments

Comments
 (0)