Merge pull request #6 from github/untrusted_checkout_improvments

untrusted checkout improvments
This commit is contained in:
Alvaro Muñoz
2024-05-06 18:37:13 +02:00
committed by GitHub
7 changed files with 21 additions and 54 deletions

View File

@@ -113,8 +113,8 @@ private predicate branchEvent(string context) {
"github\\.event\\.pull_request\\.head\\.repo\\.default_branch",
"github\\.event\\.pull_request\\.head\\.ref", "github\\.head_ref",
"github\\.event\\.workflow_run\\.head_branch",
"github\\.event\\.workflow_run\\.head_branch",
"github\\.event\\.workflow_run\\.pull_requests\\[[0-9]+\\]\\.head\\.ref",
"github\\.event\\.merge_group\\.head_ref",
]
|
Utils::normalizeExpr(context).regexpMatch(Utils::wrapRegexp(reg))
@@ -146,6 +146,7 @@ private predicate emailEvent(string context) {
"github\\.event\\.head_commit\\.committer\\.email",
"github\\.event\\.commits\\[[0-9]+\\]\\.author\\.email",
"github\\.event\\.commits\\[[0-9]+\\]\\.committer\\.email",
"github\\.event\\.merge_group\\.committer\\.email",
"github\\.event\\.workflow_run\\.head_commit\\.author\\.email",
"github\\.event\\.workflow_run\\.head_commit\\.committer\\.email",
]
@@ -165,6 +166,7 @@ private predicate usernameEvent(string context) {
"github\\.event\\.head_commit\\.committer\\.name",
"github\\.event\\.commits\\[[0-9]+\\]\\.author\\.name",
"github\\.event\\.commits\\[[0-9]+\\]\\.committer\\.name",
"github\\.event\\.merge_group\\.committer\\.name",
"github\\.event\\.workflow_run\\.head_commit\\.author\\.name",
"github\\.event\\.workflow_run\\.head_commit\\.committer\\.name",
]

View File

@@ -40,6 +40,8 @@ predicate containsHeadSHA(string s) {
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
"\\bgithub\\.event\\.check_run\\.head_sha\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.head\\.sha\\b",
"\\bgithub\\.event\\.merge_group\\.head_sha\\b",
"\\bgithub\\.event\\.merge_group\\.head_commit\\.id\\b",
// heuristics
"\\bhead\\.sha\\b", "\\bhead_sha\\b", "\\bpr_head_sha\\b"
], _, _)
@@ -56,6 +58,7 @@ predicate containsHeadRef(string s) {
"\\bgithub\\.event\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
"\\bgithub\\.event\\.check_run\\.check_suite\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
"\\bgithub\\.event\\.check_run\\.pull_requests\\[\\d+\\]\\.head\\.ref\\b",
"\\bgithub\\.event\\.merge_group\\.head_ref\\b",
// heuristics
"\\bhead\\.ref\\b", "\\bhead_ref\\b", "\\bpr_head_ref\\b",
// env vars
@@ -64,11 +67,17 @@ predicate containsHeadRef(string s) {
)
}
/** Checkout of a Pull Request HEAD ref */
/** Checkout of a Pull Request HEAD */
abstract class PRHeadCheckoutStep extends Step { }
/** Checkout of a Pull Request HEAD ref */
abstract class MutableRefCheckoutStep extends PRHeadCheckoutStep { }
/** Checkout of a Pull Request HEAD ref */
abstract class SHACheckoutStep extends PRHeadCheckoutStep { }
/** Checkout of a Pull Request HEAD ref using actions/checkout action */
class ActionsMutableRefCheckout extends PRHeadCheckoutStep instanceof UsesStep {
class ActionsMutableRefCheckout extends MutableRefCheckoutStep instanceof UsesStep {
ActionsMutableRefCheckout() {
this.getCallee() = "actions/checkout" and
(
@@ -102,7 +111,7 @@ class ActionsMutableRefCheckout extends PRHeadCheckoutStep instanceof UsesStep {
}
/** Checkout of a Pull Request HEAD ref using actions/checkout action */
class ActionsSHACheckout extends PRHeadCheckoutStep instanceof UsesStep {
class ActionsSHACheckout extends SHACheckoutStep instanceof UsesStep {
ActionsSHACheckout() {
this.getCallee() = "actions/checkout" and
(
@@ -132,7 +141,7 @@ class ActionsSHACheckout extends PRHeadCheckoutStep instanceof UsesStep {
}
/** Checkout of a Pull Request HEAD ref using git within a Run step */
class GitMutableRefCheckout extends PRHeadCheckoutStep instanceof Run {
class GitMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
GitMutableRefCheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and
@@ -154,7 +163,7 @@ class GitMutableRefCheckout extends PRHeadCheckoutStep instanceof Run {
}
/** Checkout of a Pull Request HEAD ref using git within a Run step */
class GitSHACheckout extends PRHeadCheckoutStep instanceof Run {
class GitSHACheckout extends SHACheckoutStep instanceof Run {
GitSHACheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and
@@ -173,7 +182,7 @@ class GitSHACheckout extends PRHeadCheckoutStep instanceof Run {
}
/** Checkout of a Pull Request HEAD ref using gh within a Run step */
class GhMutableRefCheckout extends PRHeadCheckoutStep instanceof Run {
class GhMutableRefCheckout extends MutableRefCheckoutStep instanceof Run {
GhMutableRefCheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and
@@ -194,7 +203,7 @@ class GhMutableRefCheckout extends PRHeadCheckoutStep instanceof Run {
}
/** Checkout of a Pull Request HEAD ref using gh within a Run step */
class GhSHACheckout extends PRHeadCheckoutStep instanceof Run {
class GhSHACheckout extends SHACheckoutStep instanceof Run {
GhSHACheckout() {
exists(string line |
this.getScript().splitAt("\n") = line and

View File

@@ -7,7 +7,7 @@
* @problem.severity error
* @precision high
* @security-severity 9.3
* @id actions/untrusted-checkout
* @id actions/privileged-untrusted-checkout/critical
* @tags actions
* security
* external/cwe/cwe-829

View File

@@ -7,7 +7,7 @@
* @problem.severity warning
* @precision medium
* @security-severity 5.3
* @id actions/untrusted-checkout
* @id actions/privileged-untrusted-checkout/high
* @tags actions
* security
* external/cwe/cwe-829

View File

@@ -1,44 +0,0 @@
# Unpinned tag for 3rd party Action in workflow
The individual jobs in a GitHub Actions workflow can interact with (and compromise) other jobs. For example, a job querying the environment variables used by a later job, writing files to a shared directory that a later job processes, or even more directly by interacting with the Docker socket and inspecting other running containers and executing commands in them. This means that a compromise of a single action within a workflow can be very significant, as that compromised action would have access to all secrets configured on your repository, and may be able to use the `GITHUB_TOKEN` to write to the repository. Consequently, there is significant risk in sourcing actions from third-party repositories on GitHub. For information on some of the steps an attacker could take, see "Security hardening for GitHub Actions."
## Recommendation
Pin an action to a full length commit SHA. This 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.
## Example
In this example, the Actions workflow uses an unpinned version.
```yaml
name: "Unpinned Action Example"
jobs:
build:
steps:
- name: Checkout repository
uses: actions-third-party-mirror/checkout@v3
- run: |
./build.sh
```
The Action is pinned in the example below.
```yaml
name: "Pinned Action Example"
jobs:
build:
steps:
- name: Checkout repository
uses: actions-mirror-third-party/checkout@ac593985615ec2ede58e132d2e21d2b1cbd6127c
- run: |
./build.sh
```
## References
- GitHub: [Security hardening for GitHub Actions](https://docs.github.com/en/actions/security-guides/security-hardening-for-github-actions)
- Common Weakness Enumeration: [CWE-829](https://cwe.mitre.org/data/definitions/829.html).