Skip to content

Commit 0110988

Browse files
KyFaStKyFaSt
authored
Merge pull request #105 from github/immutable-actions
Add CodeQL rule for Immutable actions, do not detect immutable actions in unpinned tag rule
2 parents 6a1e814 + d6e38d5 commit 0110988

15 files changed

+198
-9
lines changed

‎ql/lib/codeql/actions/config/Config.qll

+11
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,17 @@ predicate vulnerableActionsDataModel(
119119
Extensions::vulnerableActionsDataModel(action, vulnerable_version, vulnerable_sha, fixed_version)
120120
}
121121

122+
/**
123+
* MaD models for immutable actions
124+
* Fields:
125+
* - action: action name
126+
*/
127+
predicate immutableActionsDataModel(
128+
string action
129+
) {
130+
Extensions::immutableActionsDataModel(action)
131+
}
132+
122133
/**
123134
* MaD models for untrusted git commands
124135
* Fields:

‎ql/lib/codeql/actions/config/ConfigExtensions.qll

+7
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@ extensible predicate vulnerableActionsDataModel(
5858
string action, string vulnerable_version, string vulnerable_sha, string fixed_version
5959
);
6060

61+
/**
62+
* Holds for actions that are known to be immutable.
63+
*/
64+
extensible predicate immutableActionsDataModel(
65+
string action
66+
);
67+
6168
/**
6269
* Holds for git commands that may introduce untrusted data when called on an attacker controlled branch.
6370
*/

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

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
import actions
2+
3+
class UnversionedImmutableAction extends UsesStep {
4+
string immutable_action;
5+
6+
UnversionedImmutableAction() {
7+
isImmutableAction(this, immutable_action) and
8+
not isSemVer(this.getVersion())
9+
}
10+
}
11+
12+
bindingset[version]
13+
predicate isSemVer(string version) {
14+
// https://semver.org/#is-there-a-suggested-regular-expression-regex-to-check-a-semver-string with optional v prefix
15+
version.regexpMatch("^v?(0|[1-9]\\d*)\\.(0|[1-9]\\d*)\\.(0|[1-9]\\d*)(?:-((?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*)(?:\\.(?:0|[1-9]\\d*|\\d*[a-zA-Z-][0-9a-zA-Z-]*))*))?(?:\\+([0-9a-zA-Z-]+(?:\\.[0-9a-zA-Z-]+)*))?$")
16+
17+
// or N or N.x or N.N.x with optional v prefix
18+
or version.regexpMatch("^v?[1-9]\\d*$")
19+
or version.regexpMatch("^v?[1-9]\\d*\\.(x|0|([1-9]\\d*))$")
20+
or version.regexpMatch("^v?[1-9]\\d*\\.(0|([1-9]\\d*))\\.(x|0|([1-9]\\d*))$")
21+
22+
// or latest which will work
23+
or version = "latest"
24+
}
25+
26+
predicate isImmutableAction(UsesStep actionStep, string actionName) {
27+
immutableActionsDataModel(actionName) and
28+
actionStep.getCallee() = actionName
29+
}

‎ql/lib/ext/config/immutable_actions.yml

+22
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,22 @@
1+
extensions:
2+
- addsTo:
3+
pack: github/actions-all
4+
extensible: immutableActionsDataModel
5+
data:
6+
- ["actions/checkout"]
7+
- ["actions/cache"]
8+
- ["actions/setup-node"]
9+
- ["actions/upload-artifact"]
10+
- ["actions/setup-python"]
11+
- ["actions/download-artifact"]
12+
- ["actions/github-script"]
13+
- ["actions/setup-java"]
14+
- ["actions/setup-go"]
15+
- ["actions/upload-pages-artifact"]
16+
- ["actions/deploy-pages"]
17+
- ["actions/setup-dotnet"]
18+
- ["actions/stale"]
19+
- ["actions/labeler"]
20+
- ["actions/create-github-app-token"]
21+
- ["actions/configure-pages"]
22+
- ["octokit/request-action"]

‎ql/src/Security/CWE-829/UnpinnedActionsTag.md

+1-1
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@ Using a tag for a 3rd party Action that is not pinned to a commit can lead to ex
66

77
## Recommendations
88

9-
Pinning an action to a full length commit SHA is currently the only way to use an action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. When selecting a SHA, you should verify it is from the action's repository and not a repository fork.
9+
Pinning an action to a full length commit SHA is currently the only way to use a non-immutable action as an immutable release. Pinning to a particular SHA helps mitigate the risk of a bad actor adding a backdoor to the action's repository, as they would need to generate a SHA-1 collision for a valid Git object payload. When selecting a SHA, you should verify it is from the action's repository and not a repository fork.
1010

1111
## Examples
1212

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

+6-4
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
/**
2-
* @name Unpinned tag for 3rd party Action in workflow
3-
* @description Using a tag for a 3rd party Action that is not pinned to a commit can lead to executing an untrusted Action through a supply chain attack.
2+
* @name Unpinned tag for a non-immutable Action in workflow
3+
* @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
66
* @problem.severity recommendation
@@ -12,13 +12,14 @@
1212
*/
1313

1414
import actions
15+
import codeql.actions.security.UseOfUnversionedImmutableAction
1516

1617
bindingset[version]
1718
private predicate isPinnedCommit(string version) { version.regexpMatch("^[A-Fa-f0-9]{40}$") }
1819

1920
bindingset[repo]
2021
private predicate isTrustedOrg(string repo) {
21-
exists(string org | org in ["actions", "github", "advanced-security"] | repo.matches(org + "/%"))
22+
repo.matches(["actions", "github", "advanced-security"] + "/%")
2223
}
2324

2425
from UsesStep uses, string repo, string version, Workflow workflow, string name
@@ -32,7 +33,8 @@ where
3233
) and
3334
uses.getVersion() = version and
3435
not isTrustedOrg(repo) and
35-
not isPinnedCommit(version)
36+
not isPinnedCommit(version) and
37+
not isImmutableAction(uses, repo)
3638
select uses.getCalleeNode(),
3739
"Unpinned 3rd party Action '" + name + "' step $@ uses '" + repo + "' with ref '" + version +
3840
"', not a pinned commit hash", uses, uses.toString()

‎ql/src/Security/CWE-829/UnversionedImmutableAction.md

+29
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
# Unversioned Immutable Action
2+
3+
## Description
4+
5+
Using an immutable action without indicating proper semantic version will result in the version being resolved to a tag that is mutable. This means the action code can between runs and without the user's knowledge. Using an immutable action with proper semantic versioning will resolve to the exact version
6+
of the action stored in the GitHub package registry. The action code will not change between runs.
7+
8+
## Recommendations
9+
10+
When using [immutable actions](https://github.com/github/package-registry-team/blob/main/docs/immutable-actions/immutable-actions-howto.md) use the full semantic version of the action. This will ensure that the action is resolved to the exact version stored in the GitHub package registry. This will prevent the action code from changing between runs.
11+
12+
## Examples
13+
14+
### Incorrect Usage
15+
16+
```yaml
17+
- uses: actions/checkout@some-tag
18+
- uses: actions/[email protected]
19+
```
20+
21+
### Correct Usage
22+
23+
```yaml
24+
- uses: actions/[email protected]
25+
```
26+
27+
## References
28+
29+
- [Consuming immutable actions]()

‎ql/src/Security/CWE-829/UnversionedImmutableAction.ql

+19
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,19 @@
1+
/**
2+
* @name Unversioned Immutable Action
3+
* @description Using an Immutable Action without a semantic version tag opts out of the protections of Immutable Action
4+
* @kind problem
5+
* @problem.severity recommendation
6+
* @precision high
7+
* @id actions/unversioned-immutable-action
8+
* @tags security
9+
* actions
10+
* external/cwe/cwe-829
11+
*/
12+
13+
import actions
14+
import codeql.actions.security.UseOfUnversionedImmutableAction
15+
16+
from UnversionedImmutableAction step
17+
select step,
18+
"The workflow is using an eligible immutable action ($@) without semantic versioning", step,
19+
step.getCallee()

‎ql/test/query-tests/Security/CWE-829/.github/actions/dangerous-git-checkout/action.yml

+1-1
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ runs:
44
using: "composite"
55
steps:
66
- name: Checkout repo
7-
uses: actions/checkout@v4
7+
uses: actions/checkout@4
88
with:
99
ref: ${{ github.event.pull_request.head.sha }}
1010
fetch-depth: 2

‎ql/test/query-tests/Security/CWE-829/.github/workflows/issue_comment_octokit2.yml

+38
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,38 @@
1+
name: Octokit (heuristics)
2+
3+
on:
4+
issue_comment:
5+
types: [created]
6+
7+
jobs:
8+
test1:
9+
if: github.event.comment.body == '@metabase-bot run visual tests'
10+
runs-on: ubuntu-22.04
11+
steps:
12+
- name: Fetch issue
13+
uses: octokit/[email protected]
14+
id: fetch_issue
15+
with:
16+
route: GET ${{ github.event.issue.url }}
17+
env:
18+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
19+
- name: Fetch PR minor and patch wildcard
20+
uses: octokit/[email protected]
21+
id: fetch_pr
22+
with:
23+
route: GET ${{ fromJson(steps.fetch_issue.outputs.data).pull_request.url }}
24+
env:
25+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
26+
- name: Checkout PR minor patch wildcard
27+
- uses: actions/[email protected]
28+
with:
29+
ref: ${{ fromJson(steps.fetch_pr.outputs.data).head.ref }}
30+
token: ${{ secrets.GITHUB_TOKEN }}
31+
- name: Checkout PR minor wildcard incomplete patch
32+
uses: actions/[email protected].
33+
- name: Run latest action
34+
uses: some-action/some-repo@latest
35+
with:
36+
some-input: some-value
37+
- name: run the latest checkout action
38+
uses: actions/checkout@latest

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

+1-3
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,7 @@
1010
| .github/workflows/issue_comment_3rd_party_action.yml:14:15:14:52 | xt0rted/pull-request-comment-branch@v2 | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'xt0rted/pull-request-comment-branch' with ref 'v2', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:12:9:16:6 | Uses Step: comment-branch | Uses Step: comment-branch |
1111
| .github/workflows/issue_comment_3rd_party_action.yml:27:15:27:52 | xt0rted/pull-request-comment-branch@v2 | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'xt0rted/pull-request-comment-branch' with ref 'v2', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:25:9:30:6 | Uses Step: comment-branch | Uses Step: comment-branch |
1212
| .github/workflows/issue_comment_3rd_party_action.yml:41:15:41:42 | eficode/resolve-pr-refs@main | Unpinned 3rd party Action 'PR head from 3rd party action' step $@ uses 'eficode/resolve-pr-refs' with ref 'main', not a pinned commit hash | .github/workflows/issue_comment_3rd_party_action.yml:39:9:45:6 | Uses Step: refs | Uses Step: refs |
13-
| .github/workflows/issue_comment_octokit.yml:13:15:13:41 | octokit/[email protected] | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'octokit/request-action' with ref 'v2.x', not a pinned commit hash | .github/workflows/issue_comment_octokit.yml:12:9:19:6 | Uses Step: fetch_issue | Uses Step: fetch_issue |
14-
| .github/workflows/issue_comment_octokit.yml:20:15:20:41 | octokit/[email protected] | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'octokit/request-action' with ref 'v2.x', not a pinned commit hash | .github/workflows/issue_comment_octokit.yml:19:9:26:6 | Uses Step: fetch_pr | Uses Step: fetch_pr |
15-
| .github/workflows/issue_comment_octokit.yml:104:15:104:43 | octokit/[email protected] | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'octokit/request-action' with ref 'v2.0.2', not a pinned commit hash | .github/workflows/issue_comment_octokit.yml:103:9:109:6 | Uses Step: request | Uses Step: request |
13+
| .github/workflows/issue_comment_octokit2.yml:34:15:34:42 | some-action/some-repo@latest | Unpinned 3rd party Action 'Octokit (heuristics)' step $@ uses 'some-action/some-repo' with ref 'latest', not a pinned commit hash | .github/workflows/issue_comment_octokit2.yml:33:9:37:6 | Uses Step | Uses Step |
1614
| .github/workflows/label_trusted_checkout1.yml:20:13:20:36 | completely/fakeaction@v2 | Unpinned 3rd party Action 'label_trusted_checkout1.yml' step $@ uses 'completely/fakeaction' with ref 'v2', not a pinned commit hash | .github/workflows/label_trusted_checkout1.yml:20:7:24:4 | Uses Step | Uses Step |
1715
| .github/workflows/label_trusted_checkout1.yml:24:13:24:37 | fakerepo/comment-on-pr@v1 | Unpinned 3rd party Action 'label_trusted_checkout1.yml' step $@ uses 'fakerepo/comment-on-pr' with ref 'v1', not a pinned commit hash | .github/workflows/label_trusted_checkout1.yml:24:7:27:21 | Uses Step | Uses Step |
1816
| .github/workflows/label_trusted_checkout2.yml:21:13:21:36 | completely/fakeaction@v2 | Unpinned 3rd party Action 'label_trusted_checkout2.yml' step $@ uses 'completely/fakeaction' with ref 'v2', not a pinned commit hash | .github/workflows/label_trusted_checkout2.yml:21:7:25:4 | Uses Step | Uses Step |

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

+6
Original file line numberDiff line numberDiff line change
@@ -94,6 +94,12 @@ edges
9494
| .github/workflows/issue_comment_heuristic.yml:11:9:24:6 | Uses Step: get-pr-info | .github/workflows/issue_comment_heuristic.yml:24:9:28:6 | Run Step: get-sha |
9595
| .github/workflows/issue_comment_heuristic.yml:24:9:28:6 | Run Step: get-sha | .github/workflows/issue_comment_heuristic.yml:28:9:33:2 | Uses Step |
9696
| .github/workflows/issue_comment_heuristic.yml:37:7:48:4 | Run Step: vars | .github/workflows/issue_comment_heuristic.yml:48:7:50:46 | Uses Step |
97+
| .github/workflows/issue_comment_octokit2.yml:12:9:19:6 | Uses Step: fetch_issue | .github/workflows/issue_comment_octokit2.yml:19:9:26:6 | Uses Step: fetch_pr |
98+
| .github/workflows/issue_comment_octokit2.yml:19:9:26:6 | Uses Step: fetch_pr | .github/workflows/issue_comment_octokit2.yml:26:9:27:6 | name: C ... ildcard |
99+
| .github/workflows/issue_comment_octokit2.yml:26:9:27:6 | name: C ... ildcard | .github/workflows/issue_comment_octokit2.yml:27:9:31:6 | Uses Step |
100+
| .github/workflows/issue_comment_octokit2.yml:27:9:31:6 | Uses Step | .github/workflows/issue_comment_octokit2.yml:31:9:33:6 | Uses Step |
101+
| .github/workflows/issue_comment_octokit2.yml:31:9:33:6 | Uses Step | .github/workflows/issue_comment_octokit2.yml:33:9:37:6 | Uses Step |
102+
| .github/workflows/issue_comment_octokit2.yml:33:9:37:6 | Uses Step | .github/workflows/issue_comment_octokit2.yml:37:9:38:37 | Uses Step |
97103
| .github/workflows/issue_comment_octokit.yml:12:9:19:6 | Uses Step: fetch_issue | .github/workflows/issue_comment_octokit.yml:19:9:26:6 | Uses Step: fetch_pr |
98104
| .github/workflows/issue_comment_octokit.yml:19:9:26:6 | Uses Step: fetch_pr | .github/workflows/issue_comment_octokit.yml:26:9:30:6 | Uses Step |
99105
| .github/workflows/issue_comment_octokit.yml:26:9:30:6 | Uses Step | .github/workflows/issue_comment_octokit.yml:30:9:35:2 | Uses Step |

‎ql/test/query-tests/Security/CWE-829/UntrustedCheckoutHigh.expected

+1
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
| .github/workflows/issue_comment_direct.yml:43:9:46:126 | Uses Step | Potential execution of untrusted code on a privileged workflow ($@) | .github/workflows/issue_comment_direct.yml:4:3:4:15 | issue_comment | issue_comment |
66
| .github/workflows/issue_comment_heuristic.yml:28:9:33:2 | Uses Step | Potential execution of untrusted code on a privileged workflow ($@) | .github/workflows/issue_comment_heuristic.yml:4:3:4:15 | issue_comment | issue_comment |
77
| .github/workflows/issue_comment_heuristic.yml:48:7:50:46 | Uses Step | Potential execution of untrusted code on a privileged workflow ($@) | .github/workflows/issue_comment_heuristic.yml:4:3:4:15 | issue_comment | issue_comment |
8+
| .github/workflows/issue_comment_octokit2.yml:27:9:31:6 | Uses Step | Potential execution of untrusted code on a privileged workflow ($@) | .github/workflows/issue_comment_octokit2.yml:4:3:4:15 | issue_comment | issue_comment |
89
| .github/workflows/issue_comment_octokit.yml:26:9:30:6 | Uses Step | Potential execution of untrusted code on a privileged workflow ($@) | .github/workflows/issue_comment_octokit.yml:4:3:4:15 | issue_comment | issue_comment |
910
| .github/workflows/issue_comment_octokit.yml:30:9:35:2 | Uses Step | Potential execution of untrusted code on a privileged workflow ($@) | .github/workflows/issue_comment_octokit.yml:4:3:4:15 | issue_comment | issue_comment |
1011
| .github/workflows/issue_comment_octokit.yml:57:9:62:2 | Uses Step | Potential execution of untrusted code on a privileged workflow ($@) | .github/workflows/issue_comment_octokit.yml:4:3:4:15 | issue_comment | issue_comment |

0 commit comments

Comments
 (0)