Skip to content

Commit 16c6972

Browse files
fproulx-boostsecurityfproulx-boostsecurity
authored
Added new Confused Deputy Auto-Merge rule (#304)
* Added new Confused Deputy Auto-Merge rule with documentation and new utility functions * Fixed assertion in tests
1 parent dfbf385 commit 16c6972

File tree

4 files changed

+221
-0
lines changed

4 files changed

+221
-0
lines changed

‎docs/content/en/rules/confused_deputy_auto_merge.md‎

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,77 @@
1+
---
2+
title: "Confused Deputy Auto-Merge"
3+
slug: confused_deputy_auto_merge
4+
url: /rules/confused_deputy_auto_merge/
5+
rule: confused_deputy_auto_merge
6+
severity: error
7+
---
8+
9+
## Description
10+
11+
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.
12+
13+
## Remediation
14+
15+
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.
16+
17+
### GitHub Actions
18+
19+
#### Recommended
20+
21+
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.
22+
23+
For Dependabot auto-merges, consider using actions like:
24+
- `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.
25+
- `fastify/github-action-merge-dependabot`: This action is specifically designed to securely auto-merge Dependabot pull requests.
26+
- `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.
27+
28+
```yaml
29+
name: Auto-Merge Dependabot PRs
30+
31+
on:
32+
pull_request_target:
33+
types:
34+
- opened
35+
- reopened
36+
- synchronize
37+
38+
jobs:
39+
dependabot:
40+
runs-on: ubuntu-latest
41+
if: ${{ !github.event.pull_request.head.repo.fork && github.event.pull_request.user.login == 'dependabot[bot]' }}
42+
steps:
43+
- name: Auto-merge the PR
44+
run: gh pr merge --auto --squash ${{ github.event.pull_request.html_url }}
45+
env:
46+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
47+
```
48+
49+
#### Anti-Pattern
50+
51+
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.
52+
53+
```yaml
54+
name: Auto-Merge Dependabot PRs
55+
56+
on:
57+
pull_request_target:
58+
types:
59+
- opened
60+
- reopened
61+
- synchronize
62+
63+
jobs:
64+
dependabot:
65+
runs-on: ubuntu-latest
66+
if: ${{ github.actor == 'dependabot[bot]' }}
67+
steps:
68+
- name: Auto-merge the PR
69+
run: gh pr merge --auto --squash ${{ github.event.pull_request.html_url }}
70+
env:
71+
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}
72+
```
73+
74+
## See Also
75+
- [Weaponizing Dependabot: Pwn Request at its finest](https://boostsecurity.io/blog/weaponizing-dependabot-pwn-request-at-its-finest)
76+
- [GitHub Actions Exploitation: Dependabot](https://www.synacktiv.com/en/publications/github-actions-exploitation-dependabot)
77+
- [Dependabot Confusion: Gaining Access to Private GitHub Repositories using Dependabot](https://giraffesecurity.dev/posts/dependabot-confusion/)

‎opa/rego/poutine/utils.rego‎

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -74,3 +74,33 @@ to_set(xs) = xs if {
7474
} else := {v | v := xs[_]} if {
7575
is_array(xs)
7676
} else := {xs}
77+
78+
########################################################################
79+
# job order utils
80+
########################################################################
81+
82+
job_steps_after(options) := steps if {
83+
steps := {{"step": s, "step_idx": k} |
84+
s := options.job.steps[k]
85+
k > options.step_idx
86+
}
87+
}
88+
89+
job_steps_before(options) := steps if {
90+
steps := {{"step": s, "step_idx": k} |
91+
s := options.job.steps[k]
92+
k < options.step_idx
93+
}
94+
}
95+
96+
97+
########################################################################
98+
# find_first_uses_in_job
99+
########################################################################
100+
101+
find_first_uses_in_job(job, uses) := xs if {
102+
xs := {{"job": job, "step_idx": i} |
103+
s := job.steps[i]
104+
startswith(s.uses, sprintf("%v@", [uses[_]]))
105+
}
106+
}

‎opa/rego/rules/confused_deputy_auto_merge.rego‎

Lines changed: 113 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,113 @@
1+
# METADATA
2+
# title: Confused Deputy Auto-Merge
3+
# description: |-
4+
# 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.
5+
# custom:
6+
# level: error
7+
package rules.confused_deputy_auto_merge
8+
9+
import data.poutine
10+
import data.poutine.utils
11+
import rego.v1
12+
13+
merge_commands[cmd] = {
14+
"gh pr merge": `gh\s+pr\s+merge`,
15+
"gh pr review": `gh\s+pr\s+review`
16+
}[cmd]
17+
18+
merge_github_actions = {
19+
"ad-m/github-push-action",
20+
"ahmadnassri/action-dependabot-auto-merge",
21+
"ana06/automatic-pull-request-review",
22+
"endbug/add-and-commit",
23+
"hmarr/auto-approve-action",
24+
"peter-evans/create-pull-request",
25+
"stefanzweifel/git-auto-commit-action"
26+
}
27+
28+
actor_bots = {
29+
"dependabot": `dependabot\[bot\]`,
30+
"dependabot-preview": `dependabot-preview\[bot\]`,
31+
"renovate": `renovate\[bot\]`,
32+
"github-actions": `github-actions\[bot\]`
33+
}
34+
35+
uses_fix_deputy_confusion = {
36+
"dependabot/fetch-metadata",
37+
"fastify/github-action-merge-dependabot",
38+
"actions-cool/check-user-permission"
39+
}
40+
41+
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
42+
43+
rule := poutine.rule(rego.metadata.chain())
44+
45+
github.events contains event if some event in {
46+
"pull_request_target",
47+
"workflow_run"
48+
}
49+
50+
# Case with if in job
51+
results contains poutine.finding(rule, pkg_purl, {
52+
"path": workflow_path,
53+
"line": line,
54+
"details": sprintf("Detected usage of `%s` with actor `%s`", [cmd, bot]),
55+
}) if {
56+
[pkg_purl, workflow_path, job, step, cmd, line] := _merge_commands_run[_]
57+
regex.match(
58+
sprintf(regex_actor_bot, [actor_bots[bot], actor_bots[bot]]),
59+
job["if"]
60+
)
61+
}
62+
63+
# Case with if in step
64+
results contains poutine.finding(rule, pkg_purl, {
65+
"path": workflow_path,
66+
"line": line,
67+
"details": sprintf("Detected usage of `%s` with actor `%s`", [cmd, bot]),
68+
}) if {
69+
[pkg_purl, workflow_path, _, step, cmd, line] := _merge_commands_run[_]
70+
regex.match(
71+
sprintf(regex_actor_bot, [actor_bots[bot], actor_bots[bot]]),
72+
step["if"]
73+
)
74+
}
75+
76+
# Case with merge command
77+
_merge_commands_run contains [pkg_purl, workflow_path, job, step, cmd, step.lines.run] if {
78+
[pkg_purl, workflow_path, job, step] := _remove_steps_after_fetch_metadata[_]
79+
regex.match(
80+
merge_commands[cmd],
81+
step.run
82+
)
83+
}
84+
85+
# Case with github actions
86+
_merge_commands_run contains [pkg_purl, workflow_path, job, step, merge_github_action, step.line] if {
87+
[pkg_purl, workflow_path, job, step] := _remove_steps_after_fetch_metadata[_]
88+
merge_github_action := merge_github_actions[_]
89+
regex.match(
90+
merge_github_action,
91+
step.action
92+
)
93+
}
94+
95+
# Case without metadata-fetch
96+
_remove_steps_after_fetch_metadata contains [pkg.purl, workflow.path, job, s_step] if {
97+
pkg := input.packages[_]
98+
workflow := pkg.github_actions_workflows[_]
99+
job := workflow.jobs[_]
100+
relevant_steps := utils.find_first_uses_in_job(job, uses_fix_deputy_confusion)
101+
count(relevant_steps) = 0
102+
s_step = job.steps[_]
103+
}
104+
105+
# Case with metadata-fetch which fix deputy confusion problem for future steps
106+
_remove_steps_after_fetch_metadata contains [pkg.purl, workflow.path, job, s.step] if {
107+
pkg := input.packages[_]
108+
workflow := pkg.github_actions_workflows[_]
109+
job := workflow.jobs[_]
110+
relevant_steps := utils.find_first_uses_in_job(job, uses_fix_deputy_confusion)
111+
count(relevant_steps) > 0
112+
s := utils.job_steps_before(relevant_steps[_])[_]
113+
}

‎scanner/inventory_test.go‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@ func TestFindings(t *testing.T) {
9292
"debug_enabled",
9393
"job_all_secrets",
9494
"unverified_script_exec",
95+
"confused_deputy_auto_merge",
9596
})
9697

9798
findings := []results.Finding{

0 commit comments

Comments
 (0)