Skip to content

Insufficient Type Check on GraphQL leading to Maintainer delete repository

HackerOne report #858671 by ledz1996 on 2020-04-24:

Summary

As you have know, Maintainer cannot delete/archive repository. But via GraphQL, they can do as there exists an insufficient check on GraphQL API

app/graphql/mutations/snippets/destroy.rb

  def resolve(id:)  
        snippet = authorized_find!(id: id)

        response = ::Snippets::DestroyService.new(current_user, snippet).execute

The function authorized_find lead to object_from_id!

app/graphql/mutations/snippets/base.rb

  def find_object(id:)  
        GitlabSchema.object_from_id(id)  
      end

      def authorized_resource?(snippet)  
        Ability.allowed?(context[:current_user], ability_for(snippet), snippet)  
      end

      def ability_for(snippet)  
        "#{ability_name}_#{snippet.to_ability_name}".to_sym  
      end

Here there is no check for whether the Object returned from find_object is a Snippet. I could specify any object which the user have permission of
"#{ability_name}_#{snippet.to_ability_name}".to_sym to.
For example: A DiffNote that is created by a Maintainer in the Project as the function.

If I have such a ID:

mutation test{  
  destroySnippet(input: {id: "gid://gitlab/DiffNote/116"}){  
    errors  
  }  
}

It refer to an DiffNote with id 116
Perfectly, a Maintainer have an admin_note and admin_snippet on a DiffNote (!!!)

In the mutation Destroy the call to ::Snippets::DestroyService.new(current_user, snippet) but the Object of snippet is actually a DiffNote

app/services/snippets/destroy_service.rb

  def attempt_destroy!  
      result = Repositories::DestroyService.new(snippet.repository).execute

      raise DestroyError if result[:status] == :error

      snippet.destroy!  
    end

and in

app/models/diff_note.rb

  def repository  
    noteable.respond_to?(:repository) ? noteable.repository : project.repository  
  end

It return the project.repository which in turn the Project that the DestroyService gonna delete

Steps to reproduce

  1. Create 2 User: User A, User B
  2. User A create a project set User B as Maintainer.
    unauthen1.PNG
  3. User B create 2 branch with the same file but different content
    unauthen2.PNG
    unauthen3.PNG
  4. Create a merge request for those 2 Branch
    unauthen4.PNG
  5. Create a diff note for the file by clicking at the comment on a line of the file then Submit it
    unauthen5.PNG
  6. To know the ID of diff note, delete this one, the ID will show up in burp then you will know the ID of the next one
    unauthen6.PNG
  7. Use the /-/graphiql-explorer to execute the following the query
mutation test{  
  destroySnippet(input: {id: "gid://gitlab/DiffNote/118"}){  
    errors  
  }  
}

unauthen7.PNG
8. Enjoy no repository
unauthen8.PNG
9. If you click on Create empty repository It will actually make the Project 404 but It still show up on the User's project feed
unauthen10.PNG
unauthen9.PNG

Impact

Unauthorized deleting of repository/project by maintainers

Output of checks

This bug happens on GitLab.com

Impact

Unauthorized deleting of repository/project by maintainers

Attachments

Warning: Attachments received through HackerOne, please exercise caution!

Edited by Brett Walker