Skip to content

Commit 06c9087

Browse files
authored
Merge branch 'main' into redsun82/issue-21802-ruby-absolute-paths-in-sarif-diagnostics-a02887
2 parents c2fc0cf + 22a8123 commit 06c9087

41 files changed

Lines changed: 16089 additions & 4587 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

actions/ql/examples/snippets/uses_pinned_sha.ql

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,5 +8,5 @@
88
import actions
99

1010
from UsesStep uses
11-
where uses.getVersion().regexpMatch("^[A-Fa-f0-9]{40}$")
11+
where uses.getVersion().regexpMatch("^[A-Fa-f0-9]{40}([A-Fa-f0-9]{24})?$")
1212
select uses, "This 'uses' step has a pinned SHA version."
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 GitHub Actions analysis now recognizes more Bash regex checks that restrict a value to alphanumeric characters, include regexes like `^[0-9a-zA-Z]{40}([0-9a-zA-Z]{24})?$` which check for a sha1 or sha256 hash. This may reduce false positive results where command output is validated with grouped or optional alphanumeric patterns before being used.

actions/ql/lib/codeql/actions/Bash.qll

Lines changed: 17 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -785,7 +785,22 @@ module Bash {
785785

786786
/**
787787
* Holds if the given regex is used to match an alphanumeric string
788-
* eg: `^[0-9a-zA-Z]{40}$`, `^[0-9]+$` or `^[a-zA-Z0-9_]+$`
788+
* eg: `^[0-9a-zA-Z]{40}([0-9a-zA-Z]{24})?$`, `^[0-9]+$` or `^[a-zA-Z0-9_]+$`
789789
*/
790-
string alphaNumericRegex() { result = "^\\^\\[([09azAZ_-]+)\\](\\+|\\{\\d+\\})\\$$" }
790+
string alphaNumericRegex() {
791+
exists(string r1, string r2, string r3, string r4 |
792+
// An alphanumeric character class
793+
r1 = "\\[([09azAZ_-]+)\\]" and
794+
// The same as above, followed by a quantifier like `+` or `{20}`
795+
r2 = r1 + "(\\+|\\{\\d+\\})" and
796+
// The same as above, possibly with parentheses around it
797+
r3 = "\\(?" + r2 + "\\)?" and
798+
// The same as above, possibly with a `?` after it
799+
r4 = r3 + "\\??"
800+
|
801+
// The same as above, repeated one or more times, and with `^` at the
802+
// beginning and `$` at the end
803+
result = "^\\^(" + r4 + ")+\\$$"
804+
)
805+
}
791806
}

actions/ql/src/Security/CWE-829/UnpinnedActionsTag.ql

Lines changed: 22 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/**
2-
* @name Unpinned tag for a non-immutable Action in workflow
2+
* @name Unpinned tag for a non-immutable Action in workflow or composite action
33
* @description Using a tag for a non-immutable Action that is not pinned to a commit can lead to executing an untrusted Action through a supply chain attack.
44
* @kind problem
55
* @security-severity 5.0
@@ -15,7 +15,9 @@ import actions
1515
import codeql.actions.security.UseOfUnversionedImmutableAction
1616

1717
bindingset[version]
18-
private predicate isPinnedCommit(string version) { version.regexpMatch("^[A-Fa-f0-9]{40}$") }
18+
private predicate isPinnedCommit(string version) {
19+
version.regexpMatch("^[A-Fa-f0-9]{40}([A-Fa-f0-9]{24})?$")
20+
}
1921

2022
bindingset[nwo]
2123
private predicate isTrustedOwner(string nwo) {
@@ -31,15 +33,26 @@ private predicate isPinnedContainer(string version) {
3133
bindingset[nwo]
3234
private predicate isContainerImage(string nwo) { nwo.regexpMatch("^docker://.+") }
3335

34-
from UsesStep uses, string nwo, string version, Workflow workflow, string name
36+
private predicate getStepContainerName(UsesStep uses, string name) {
37+
exists(Workflow workflow |
38+
uses.getEnclosingWorkflow() = workflow and
39+
(
40+
workflow.getName() = name
41+
or
42+
not exists(workflow.getName()) and workflow.getLocation().getFile().getBaseName() = name
43+
)
44+
)
45+
or
46+
exists(CompositeAction action |
47+
uses.getEnclosingCompositeAction() = action and
48+
name = action.getLocation().getFile().getBaseName()
49+
)
50+
}
51+
52+
from UsesStep uses, string nwo, string version, string name
3553
where
3654
uses.getCallee() = nwo and
37-
uses.getEnclosingWorkflow() = workflow and
38-
(
39-
workflow.getName() = name
40-
or
41-
not exists(workflow.getName()) and workflow.getLocation().getFile().getBaseName() = name
42-
) and
55+
getStepContainerName(uses, name) and
4356
uses.getVersion() = version and
4457
not isTrustedOwner(nwo) and
4558
not (if isContainerImage(nwo) then isPinnedContainer(version) else isPinnedCommit(version)) and
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/unpinned-tag` query now analyzes composite action metadata (`action.yml`/`action.yaml` files) in addition to workflow files, providing more comprehensive detection of unpinned action references across the entire Actions ecosystem.
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/unpinned-tag` query now recognizes 64-character SHA-256 commit hashes as properly pinned references, in addition to 40-character SHA-1 hashes.
Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
name: Composite unpinned tag test
2+
runs:
3+
using: "composite"
4+
steps:
5+
- uses: foo/bar@v2
6+
- uses: foo/bar@25b062c917b0c75f8b47d8469aff6c94ffd89abb

actions/ql/test/query-tests/Security/CWE-829/.github/workflows/unpinned_tags.yml

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,3 +11,9 @@ jobs:
1111
- uses: foo/bar@25b062c917b0c75f8b47d8469aff6c94ffd89abb
1212
- uses: docker://foo/bar@latest
1313
- uses: docker://foo/bar@sha256:887a259a5a534f3c4f36cb02dca341673c6089431057242cdc931e9f133147e9
14+
# SHA-256 pinned (64 hex chars) - should NOT be flagged
15+
- uses: foo/bar@25b062c917b0c75f8b47d8469aff6c94ffd89abb25b062c917b0c75f8b47d84d
16+
# SHA-1 pinned (40 hex chars) regression - should NOT be flagged
17+
- uses: foo/bar@a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2
18+
# Invalid 50-char hex string - should be flagged
19+
- uses: foo/bar@a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2a1b2c3d4e5

actions/ql/test/query-tests/Security/CWE-829/UnpinnedActionsTag.expected

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
| .github/actions/unpinned-tag/action.yml:5:13:5:22 | foo/bar@v2 | Unpinned 3rd party Action 'action.yml' step $@ uses 'foo/bar' with ref 'v2', not a pinned commit hash | .github/actions/unpinned-tag/action.yml:5:7:6:4 | Uses Step | Uses Step |
12
| .github/workflows/actor_trusted_checkout.yml:19:13:19:36 | completely/fakeaction@v2 | Unpinned 3rd party Action 'actor_trusted_checkout.yml' step $@ uses 'completely/fakeaction' with ref 'v2', not a pinned commit hash | .github/workflows/actor_trusted_checkout.yml:19:7:23:4 | Uses Step | Uses Step |
23
| .github/workflows/actor_trusted_checkout.yml:23:13:23:37 | fakerepo/comment-on-pr@v1 | Unpinned 3rd party Action 'actor_trusted_checkout.yml' step $@ uses 'fakerepo/comment-on-pr' with ref 'v1', not a pinned commit hash | .github/workflows/actor_trusted_checkout.yml:23:7:26:21 | Uses Step | Uses Step |
34
| .github/workflows/artifactpoisoning21.yml:13:15:13:49 | dawidd6/action-download-artifact@v2 | Unpinned 3rd party Action 'Pull Request Open' step $@ uses 'dawidd6/action-download-artifact' with ref 'v2', not a pinned commit hash | .github/workflows/artifactpoisoning21.yml:13:9:18:6 | Uses Step | Uses Step |
@@ -33,3 +34,4 @@
3334
| .github/workflows/test18.yml:37:21:37:63 | sonarsource/sonarcloud-github-action@master | Unpinned 3rd party Action 'Sonar' step $@ uses 'sonarsource/sonarcloud-github-action' with ref 'master', not a pinned commit hash | .github/workflows/test18.yml:36:15:40:58 | Uses Step | Uses Step |
3435
| .github/workflows/unpinned_tags.yml:10:13:10:22 | foo/bar@v1 | Unpinned 3rd party Action 'unpinned_tags.yml' step $@ uses 'foo/bar' with ref 'v1', not a pinned commit hash | .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | Uses Step |
3536
| .github/workflows/unpinned_tags.yml:12:13:12:35 | docker://foo/bar@latest | Unpinned 3rd party Action 'unpinned_tags.yml' step $@ uses 'docker://foo/bar' with ref 'latest', not a pinned commit hash | .github/workflows/unpinned_tags.yml:12:7:13:4 | Uses Step | Uses Step |
37+
| .github/workflows/unpinned_tags.yml:19:13:19:70 | foo/bar@a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2a1b2c3d4e5 | Unpinned 3rd party Action 'unpinned_tags.yml' step $@ uses 'foo/bar' with ref 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2a1b2c3d4e5', not a pinned commit hash | .github/workflows/unpinned_tags.yml:19:7:19:71 | Uses Step | Uses Step |

actions/ql/test/query-tests/Security/CWE-829/UntrustedCheckoutCritical.expected

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ edges
88
| .github/actions/download-artifact/action.yaml:25:7:29:4 | Run Step | .github/actions/download-artifact/action.yaml:29:7:32:18 | Run Step |
99
| .github/actions/download-artifact/action.yaml:29:7:32:18 | Run Step | .github/workflows/artifactpoisoning91.yml:19:9:25:6 | Run Step: metadata |
1010
| .github/actions/download-artifact/action.yaml:29:7:32:18 | Run Step | .github/workflows/resolve-args.yml:22:9:36:13 | Run Step: resolve-step |
11+
| .github/actions/unpinned-tag/action.yml:5:7:6:4 | Uses Step | .github/actions/unpinned-tag/action.yml:6:7:6:61 | Uses Step |
1112
| .github/workflows/actor_trusted_checkout.yml:9:7:14:4 | Uses Step | .github/workflows/actor_trusted_checkout.yml:14:7:15:4 | Uses Step |
1213
| .github/workflows/actor_trusted_checkout.yml:14:7:15:4 | Uses Step | .github/workflows/actor_trusted_checkout.yml:15:7:19:4 | Run Step |
1314
| .github/workflows/actor_trusted_checkout.yml:15:7:19:4 | Run Step | .github/workflows/actor_trusted_checkout.yml:19:7:23:4 | Uses Step |
@@ -311,7 +312,10 @@ edges
311312
| .github/workflows/unpinned_tags.yml:9:7:10:4 | Uses Step | .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step |
312313
| .github/workflows/unpinned_tags.yml:10:7:11:4 | Uses Step | .github/workflows/unpinned_tags.yml:11:7:12:4 | Uses Step |
313314
| .github/workflows/unpinned_tags.yml:11:7:12:4 | Uses Step | .github/workflows/unpinned_tags.yml:12:7:13:4 | Uses Step |
314-
| .github/workflows/unpinned_tags.yml:12:7:13:4 | Uses Step | .github/workflows/unpinned_tags.yml:13:7:13:101 | Uses Step |
315+
| .github/workflows/unpinned_tags.yml:12:7:13:4 | Uses Step | .github/workflows/unpinned_tags.yml:13:7:15:4 | Uses Step |
316+
| .github/workflows/unpinned_tags.yml:13:7:15:4 | Uses Step | .github/workflows/unpinned_tags.yml:15:7:17:4 | Uses Step |
317+
| .github/workflows/unpinned_tags.yml:15:7:17:4 | Uses Step | .github/workflows/unpinned_tags.yml:17:7:19:4 | Uses Step |
318+
| .github/workflows/unpinned_tags.yml:17:7:19:4 | Uses Step | .github/workflows/unpinned_tags.yml:19:7:19:71 | Uses Step |
315319
| .github/workflows/untrusted_checkout2.yml:7:9:14:6 | Run Step: pr_number | .github/workflows/untrusted_checkout2.yml:14:9:19:72 | Run Step |
316320
| .github/workflows/untrusted_checkout3.yml:11:9:12:6 | Uses Step | .github/workflows/untrusted_checkout3.yml:12:9:13:6 | Uses Step |
317321
| .github/workflows/untrusted_checkout3.yml:12:9:13:6 | Uses Step | .github/actions/dangerous-git-checkout/action.yml:6:7:11:4 | Uses Step |

0 commit comments

Comments
 (0)