Developers can bypass code owners approval by changing a MR's base branch
HackerOne report #2109371 by js_noob
on 2023-08-14, assigned to @nmalcolm:
Report | Attachments | How To Reproduce
Report
Summary
Hello team, a project owner can set code owners in his project, the code owners' approvals are required on some branches if it was set by the owner. For example, an owner can set 3 users as code owners and require their approval on MRs that has a base branch as main
. However, developers can bypass this by by abusing some really weird behavior happening, where if a code owner approved a specific MR the developer can change the base branch to main
(which should explicitly require approvals) and the approvals would persist allowing him to merge this code, that should be approved to be merged on a branch other than main
.
Steps to reproduce
As an owner:
- Create a new group and apply the ultimate trial to it
- Create a new project in that project
- Create a
CODEOWNERS
with the following content, but replaceYOUR_USERNAME
with your username
[Code Owners]
* [@]YOUR_USERNAME
- Navigate to Project settings => Repository => Protected branches, and allow
Developers + Maintainers
to merge tomain
, and also toggle onCode owner approval
- Navigate to Project settings => Merge requests => Approval settings, check
Prevent approval by author
,Prevent approvals by users who add commits
, andPrevent editing approval rules in merge requests
and underWhen a commit is added:
selectRemove approvals by Code Owners if their files changed
- Invite a developer to the group
As the developer:
- Create a new branch from
main
calleddelete-CODEOWNERS
and delete theCODEOWNERS
file (or you can do that in a single step it doesn't matter) - Create another branch from
main
let's callmy-branch
and add any change to theREADME.md
file - From that branch create a MR with
delete-CODEOWNERS
as the base branch
As the owner:
- Approve that newly created MR (at this point the owner is okay to merge these changes into
delete-CODEOWNERS
only and notmain
)
As the developer:
- Change the base branch of the MR to
main
, and verify that the approval persists - Merge the MR into
main
, and verify the success
NOTE: The above bug doesn't work unless the "initial" base branch contains a commit that deletes the CODEOWNERS
file, so I am pretty sure there's some weird edge case here leading to this.
Examples/POC
bandicam_2023-08-14_20-04-41-269.mp4
What is the current bug behavior?
Changing the base branch of a MR persists code owners' approvals.
What is the expected correct behavior?
Changing the base branch of a MR should remove code owners' approvals.
Output of checks
This bug happens on GitLab.com
Impact
Unauthorized content is being pushed to protected branches, with impact varying depending on:
- Protected GitLab CI/CD variables content
- Automated CI/CD
Attachments
Warning: Attachments received through HackerOne, please exercise caution!
How To Reproduce
Please add reproducibility information to this section: