Skip to content

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

  1. Fork a project
  2. Make some changes and open a MR to the upstream project
  3. 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

image

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)

Edited by Niklas van Schrick