Non-Member can update MR Assignees of own created MRs
Summary
A user can set assignees and reviewers on merge requests that are created by them in projects where they have no membership
Steps to reproduce
- Fork a project
- Make some changes and open a MR to the upstream project
curl -X PUT -H 'Private-Token:[TOKEN]' https://gitlab.com/api/v4/projects/<UPSTREAM-PROJECT-ID>/merge_requests/<MR-ID>?assignee_ids[]=<ANY-USER-ID>
Example Project
leipert-projects/test-project!1 (closed) (Thanks @leipert for looking through the code with me and allowing me to use the test project for this)
I'm not a member of the project (https://gitlab.com/leipert-projects/test-project/-/project_members) and also can't change assignees with the UI
What is the current bug behavior?
Users without project membership can assign any user to a MR that they created themself.
This only works if only assignee_id
or assignee_ids
parameters are used. Once you include another parameter like labels
, the assignees will not change.
What is the expected correct behavior?
As I can't change the assignee in the UI, I would expect that it is also not possible with the API.
Relevant logs and/or screenshots
Output of checks
This bug happens on GitLab.com
Results of GitLab environment info
Expand for output related to GitLab environment info
(For installations with omnibus-gitlab package run and paste the output of: `sudo gitlab-rake gitlab:env:info`) (For installations from source run and paste the output of: `sudo -u git -H bundle exec rake gitlab:env:info RAILS_ENV=production`)
Results of GitLab application Check
Expand for output related to the GitLab application check
(For installations with omnibus-gitlab package run and paste the output of:
sudo gitlab-rake gitlab:check SANITIZE=true
)(For installations from source run and paste the output of:
sudo -u git -H bundle exec rake gitlab:check RAILS_ENV=production SANITIZE=true
)(we will only investigate if the tests are passing)
Possible fixes
MergeRequests::UpdateAssigneesService
checks for :update_merge_request
.
This ability is given to authors and assignees here: https://gitlab.com/gitlab-org/gitlab/-/blob/2c3dc38890cf7f8f1c3b9431c56da502b19694da/app/policies/issuable_policy.rb#L28
One thing to consider is MergeRequests::UpdateReviewersService
also checks for :update_merge_request
.
This service is not affected by this if the REST API is used.
It is however affected when the mergeRequestSetReviewers
GraphQL mutation is used. The mergeRequestSetAssignees
mutation works as well.
A fix could be to change these services to check for :set_merge_request_metadata
instead of :update_merge_request
, but it could be considered a breaking change as it makes the required permissions more strict.
cc @gitlab-com/gl-security/appsec (I'm not sure if this is really a bugvulnerability, I will let you decide that)