-
Notifications
You must be signed in to change notification settings - Fork 40
Added new Confused Deputy Auto-Merge rule #304
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,77 @@ | ||
| --- | ||
| title: "Confused Deputy Auto-Merge" | ||
| slug: confused_deputy_auto_merge | ||
| url: /rules/confused_deputy_auto_merge/ | ||
| rule: confused_deputy_auto_merge | ||
| severity: error | ||
| --- | ||
|
|
||
| ## Description | ||
|
|
||
| The workflow appears to be vulnerable to a "Confused Deputy" attack when processing Pull Requests, typically from bots like Dependabot. This vulnerability occurs when a workflow triggered by an event like `pull_request_target` automatically trusts an actor (e.g., `github.actor == 'dependabot[bot]'`) to perform privileged actions, such as merging a Pull Request. An attacker can exploit this by tricking the trusted bot (the "confused deputy") into triggering the workflow on a Pull Request from a fork containing malicious changes. The workflow then mistakenly executes the privileged action (e.g., auto-merging the malicious code) because it only checks the identity of the bot that triggered the event, not guaranteeing the origin of the code changes within the Pull Request. | ||
|
|
||
| ## Remediation | ||
|
|
||
| The core principle for remediation is to ensure that any automated action, especially merging, is based on a reliable verification of the content and origin of the Pull Request, rather than solely on the actor triggering the workflow event. | ||
|
|
||
| ### GitHub Actions | ||
|
|
||
| #### Recommended | ||
|
|
||
| Instead of directly using `github.actor` to authorize merge operations in a `pull_request_target` workflow, use specialized GitHub Actions designed to securely handle dependency update PRs or verify the true initiator of the changes. These actions often inspect the PR metadata or commit history to confirm that the changes are genuinely from the trusted bot and not manipulated by an attacker. | ||
|
|
||
| For Dependabot auto-merges, consider using actions like: | ||
| - `dependabot/fetch-metadata`: This action helps you reliably determine metadata about a Dependabot PR, including whether it has been edited by a user. You can then use this information in subsequent steps to make a safer merge decision. | ||
| - `fastify/github-action-merge-dependabot`: This action is specifically designed to securely auto-merge Dependabot pull requests. | ||
| - `actions-cool/check-user-permission`: This action can be used to verify permissions of the user who created the pull request or initiated the changes, rather than just the `github.actor` of the current workflow run. | ||
|
|
||
| ```yaml | ||
| name: Auto-Merge Dependabot PRs | ||
|
|
||
| on: | ||
| pull_request_target: | ||
| types: | ||
| - opened | ||
| - reopened | ||
| - synchronize | ||
|
|
||
| jobs: | ||
| dependabot: | ||
| runs-on: ubuntu-latest | ||
| if: ${{ !github.event.pull_request.head.repo.fork && github.event.pull_request.user.login == 'dependabot[bot]' }} | ||
| steps: | ||
| - name: Auto-merge the PR | ||
| run: gh pr merge --auto --squash ${{ github.event.pull_request.html_url }} | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| ``` | ||
|
|
||
| #### Anti-Pattern | ||
|
|
||
| This example demonstrates a common anti-pattern where a workflow triggered on `pull_request_target` relies solely on `github.actor` to identify a bot (like Dependabot) and then proceeds to automatically merge the Pull Request. | ||
|
|
||
| ```yaml | ||
| name: Auto-Merge Dependabot PRs | ||
|
|
||
| on: | ||
| pull_request_target: | ||
| types: | ||
| - opened | ||
| - reopened | ||
| - synchronize | ||
|
|
||
| jobs: | ||
| dependabot: | ||
| runs-on: ubuntu-latest | ||
| if: ${{ github.actor == 'dependabot[bot]' }} | ||
| steps: | ||
| - name: Auto-merge the PR | ||
| run: gh pr merge --auto --squash ${{ github.event.pull_request.html_url }} | ||
| env: | ||
| GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} | ||
| ``` | ||
|
|
||
| ## See Also | ||
| - [Weaponizing Dependabot: Pwn Request at its finest](https://boostsecurity.io/blog/weaponizing-dependabot-pwn-request-at-its-finest) | ||
| - [GitHub Actions Exploitation: Dependabot](https://www.synacktiv.com/en/publications/github-actions-exploitation-dependabot) | ||
| - [Dependabot Confusion: Gaining Access to Private GitHub Repositories using Dependabot](https://giraffesecurity.dev/posts/dependabot-confusion/) |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| # METADATA | ||
| # title: Confused Deputy Auto-Merge | ||
| # description: |- | ||
| # Confused Deputy for GitHub Actions is a situation where a GitHub event attribute (ex. github.actor) is used to check the last interaction of a certain event. This allows an attacker abuse an event triggered by a Bot (ex. @dependabot recreate) and trigger as a side effect other privileged workflows, which may for instance automatically merge unapproved changes. | ||
| # custom: | ||
| # level: error | ||
| package rules.confused_deputy_auto_merge | ||
|
|
||
| import data.poutine | ||
| import data.poutine.utils | ||
| import rego.v1 | ||
|
|
||
| merge_commands[cmd] = { | ||
| "gh pr merge": `gh\s+pr\s+merge`, | ||
| "gh pr review": `gh\s+pr\s+review` | ||
| }[cmd] | ||
|
|
||
| merge_github_actions = { | ||
| "ad-m/github-push-action", | ||
| "ahmadnassri/action-dependabot-auto-merge", | ||
| "ana06/automatic-pull-request-review", | ||
| "endbug/add-and-commit", | ||
| "hmarr/auto-approve-action", | ||
| "peter-evans/create-pull-request", | ||
| "stefanzweifel/git-auto-commit-action" | ||
| } | ||
|
|
||
| actor_bots = { | ||
| "dependabot": `dependabot\[bot\]`, | ||
| "dependabot-preview": `dependabot-preview\[bot\]`, | ||
| "renovate": `renovate\[bot\]`, | ||
| "github-actions": `github-actions\[bot\]` | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Worth including
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Thanks for the head up ! |
||
| } | ||
|
|
||
| uses_fix_deputy_confusion = { | ||
| "dependabot/fetch-metadata", | ||
| "fastify/github-action-merge-dependabot", | ||
| "actions-cool/check-user-permission" | ||
| } | ||
|
|
||
| regex_actor_bot = `(github\.(actor|triggering_actor)\s*==\s*\'%v\'|contains\(\s*fromJSON\(.*%v.*\)\s*,\s*github\.(actor|triggering_actor)\s*\))` # Unit test: https://regex101.com/r/tjnx0o/1 | ||
|
|
||
| rule := poutine.rule(rego.metadata.chain()) | ||
|
|
||
| github.events contains event if some event in { | ||
| "pull_request_target", | ||
| "workflow_run" | ||
| } | ||
|
|
||
| # Case with if in job | ||
| results contains poutine.finding(rule, pkg_purl, { | ||
| "path": workflow_path, | ||
| "line": line, | ||
| "details": sprintf("Detected usage of `%s` with actor `%s`", [cmd, bot]), | ||
| }) if { | ||
| [pkg_purl, workflow_path, job, step, cmd, line] := _merge_commands_run[_] | ||
|
fproulx-boostsecurity marked this conversation as resolved.
|
||
| regex.match( | ||
| sprintf(regex_actor_bot, [actor_bots[bot], actor_bots[bot]]), | ||
| job["if"] | ||
| ) | ||
| } | ||
|
|
||
| # Case with if in step | ||
| results contains poutine.finding(rule, pkg_purl, { | ||
| "path": workflow_path, | ||
| "line": line, | ||
| "details": sprintf("Detected usage of `%s` with actor `%s`", [cmd, bot]), | ||
| }) if { | ||
| [pkg_purl, workflow_path, _, step, cmd, line] := _merge_commands_run[_] | ||
| regex.match( | ||
| sprintf(regex_actor_bot, [actor_bots[bot], actor_bots[bot]]), | ||
| step["if"] | ||
| ) | ||
| } | ||
|
|
||
| # Case with merge command | ||
| _merge_commands_run contains [pkg_purl, workflow_path, job, step, cmd, step.lines.run] if { | ||
| [pkg_purl, workflow_path, job, step] := _remove_steps_after_fetch_metadata[_] | ||
| regex.match( | ||
| merge_commands[cmd], | ||
| step.run | ||
| ) | ||
| } | ||
|
|
||
| # Case with github actions | ||
| _merge_commands_run contains [pkg_purl, workflow_path, job, step, merge_github_action, step.line] if { | ||
| [pkg_purl, workflow_path, job, step] := _remove_steps_after_fetch_metadata[_] | ||
| merge_github_action := merge_github_actions[_] | ||
| regex.match( | ||
| merge_github_action, | ||
| step.action | ||
| ) | ||
| } | ||
|
|
||
| # Case without metadata-fetch | ||
| _remove_steps_after_fetch_metadata contains [pkg.purl, workflow.path, job, s_step] if { | ||
| pkg := input.packages[_] | ||
| workflow := pkg.github_actions_workflows[_] | ||
| job := workflow.jobs[_] | ||
| relevant_steps := utils.find_first_uses_in_job(job, uses_fix_deputy_confusion) | ||
| count(relevant_steps) = 0 | ||
| s_step = job.steps[_] | ||
| } | ||
|
|
||
| # Case with metadata-fetch which fix deputy confusion problem for future steps | ||
| _remove_steps_after_fetch_metadata contains [pkg.purl, workflow.path, job, s.step] if { | ||
| pkg := input.packages[_] | ||
| workflow := pkg.github_actions_workflows[_] | ||
| job := workflow.jobs[_] | ||
| relevant_steps := utils.find_first_uses_in_job(job, uses_fix_deputy_confusion) | ||
| count(relevant_steps) > 0 | ||
| s := utils.job_steps_before(relevant_steps[_])[_] | ||
| } | ||
Uh oh!
There was an error while loading. Please reload this page.