Approved merge request can introduce extra unverified changes without requiring a re-approve
HackerOne report #1767639 by zzyzxd
on 2022-11-09, assigned to Ottilia Westerlund
:
Report
NOTE! Thanks for submitting a report! Please note that initial triage is handled by HackerOne staff. They are identified with a
HackerOne triage
badge and will escalate to the GitLab team any. Please replace all the (parenthesized) sections below with the pertinent details. Remember, the more detail you provide, the easier it is for us to triage and respond quickly, so be sure to take your time filling out the report!
Summary
GitLab Premium/Ultimate tiers allow users to enable "approval settings" for a merge request (MR), such as "when a commit is added, remove all existing approvals". However, after pushing a new commit to the MR's source branch, it usually takes several seconds before the approvals are removed by the system. Within this window, a user with sufficient permission to merge can merge the merge request with zero approval. This gives the user ability to introduce arbitrary to a protected branch without project owner's review/approval.
Steps to reproduce
(Step-by-step guide to reproduce the issue, including:)
This has been successfully reproduced in my public GitLab project: https://gitlab.com/yuha0/xxx, specifically in merge request !3. Below is a description of what happened in this merge request:
- Identify a project with "when a commit is added, remove all existing approvals" box checked in project settings. My project, https://gitlab.com/yuha0/xxx, is one.
- Make a legit merge request(in this sample, it's !3).
- Wait until the merge request is reviewed and approved by one of the eligible approvers.
- Push another commit to the MR source branch
- Refresh the MR webpage, observe that the new commit has been displayed in the UI (The "Activity" section will display something like "XXX added 2 commits 5 seconds ago").
- Within a few seconds, the existing approval from step 3 continues to be presented, and the merge button is still available to users with enough permission to merge. If one of them click "merge", the MR will be merged successfully. If no one click merge, after about 5-10 seconds, the approval from step 3 will be removed.
Impact
The GitLab feature "when a commit is added, remove all existing approvals" can be easily bypassed. Once an MR is approved, a malicious party with permission to write to the MR source branch can update the MR further with any arbitrary changes they want, and merge it into a protected branch without getting further reviewed.
Examples
As mentioned above, here's an example of a success attack in my personal project: yuha0/xxx!3 (merged)
Please take a look at the Activity section in the MR:
- When the MR was created in the first place, it only contained one commit, which creates a new empty file,
xxx
: yuha0/xxx!3 (a028e131) - I, as a eligible approver, approved the MR
- I pushed another commit, which creates another empty file,
yyy
: yuha0/xxx!3 (436984e8) - After step 3, I refreshed the webpage, and clicked the "merge" button. Then this MR was successfully merged. No re-approval required.
Notes:
I did enable "when a commit is added, remove all existing approvals". As a proof, in a separate MR, !2:
- I approved the MR
- Pushed new commits
- Did not click merge and waited for 10 seconds
- The existing approval was removed by the system, just like what it was designed to do.
What is the current bug behavior?
After adding new commits to the MR, the system takes 5-10 seconds before it removes the existing approval. Within the window, the approval is still valid and merge is permitted.
What is the expected correct behavior?
After adding new commits to the MR, the system should immediately remove approvals and reject merge operations.
Relevant logs and/or screenshots
N/A
Output of checks
- This bug has been successfully reproduced on gitlab.com on Nov 8th between 9:49PM and 9:50PM PT.
- This bug has been successfully reproduced on a self-hosted GitLab instance with Premium license, version:
15.4.4-ee
Results of GitLab environment info
See "Output of checks" above.
Impact
Even with "when a commit is added, remove all existing approvals" feature activated, an attacker can merge commits into protected branches without receiving sufficient approvals.
How To Reproduce
Please add reproducibility information to this section: