bugzilla.mozilla.org has resumed normal operation. Attachments prior to 2014 will be unavailable for a few days. This is tracked in Bug 1475801.
Please report any other irregularities here.

Any logged in user can delete another user's API tokens

RESOLVED FIXED

Status

Tree Management
Treeherder: API
P1
normal
RESOLVED FIXED
2 years ago
a year ago

People

(Reporter: Griffin Francis, Assigned: wlach)

Tracking

({sec-high, wsec-authorization})

sec-high, wsec-authorization
Dependency tree / graph
Bug Flags:
sec-bounty +

Details

Attachments

(6 attachments)

(Reporter)

Description

2 years ago
Created attachment 8792747 [details]
Example One.PNG

User Agent: Mozilla/5.0 (Windows NT 6.3; WOW64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/53.0.2785.116 Safari/537.36

Steps to reproduce:

Login via Persona at the URL - https://treeherder.mozilla.org/credentials/

You should now be able to login and delete users at links such as the following https://treeherder.mozilla.org/credentials/4/delete/ & https://treeherder.mozilla.org/credentials/9/delete/. You can change the number around until you find a user which is valid.

Due to the sensitivity of the accounts at hand here I haven't clicked "Confirm delete". I am using the request will authorize at this point in time.


Actual results:

Possibility of being able to delete users associated with the application.


Expected results:

Requests should be validated.
(Reporter)

Comment 1

2 years ago
Created attachment 8792748 [details]
Example Two.PNG
(Reporter)

Comment 2

2 years ago
Created attachment 8792749 [details]
Example Three.PNG
(Reporter)

Comment 3

2 years ago
I have confirmed this on Allizom with permission of Jonathan. Attached are more screenshots.
(Reporter)

Comment 4

2 years ago
Created attachment 8792753 [details]
Before Delete UID 13 Allizom.PNG
(Reporter)

Comment 5

2 years ago
Created attachment 8792754 [details]
After Delete UID 13 Allizom.PNG
Ed - Could you please have an (urgent) look at this issue?
Assignee: nobody → emorley
Status: UNCONFIRMED → ASSIGNED
Component: Other → Tools
Ever confirmed: true
Flags: sec-bounty?
Keywords: sec-high, wsec-authorization
Product: Websites → Release Engineering
QA Contact: hwine

Updated

2 years ago
Group: websites-security → webtools-security
Component: Tools → Treeherder: API
Priority: -- → P1
Product: Release Engineering → Tree Management
QA Contact: hwine
Version: unspecified → ---

Comment 7

2 years ago
Confirmed. A standard user is able to delete the API credentials of any other user, whereas this should be limited to either (a) admins, (b) the credential owner.

Attempting to view the other user's API token does not work however. eg:
https://treeherder.mozilla.org/credentials/2/

Comment 8

2 years ago
So the list/detail views filter by the owner before returning, so return a 404 if the ID belonging to someone else is used.

However the delete endpoint has no filtering:
https://github.com/mozilla/treeherder/blob/master/treeherder/credentials/views.py
Created attachment 8793060 [details] [diff] [review]
Patch

This seems to fix things for me locally. Does it look ok Ed? Ideally we'd have a unit test for this as well.

Honestly this credentials stuff looks pretty brittle to me, fortunately there isn't too much of it.
Attachment #8793060 - Flags: review?(emorley)
Comment on attachment 8793060 [details] [diff] [review]
Patch

I think actually limiting to just the owner should be fine, since admins never use this endpoint for other people's credentials, and instead use the admin panel.

However happy for this to land as-is either way :-)
Attachment #8793060 - Attachment is patch: true
Attachment #8793060 - Flags: review?(emorley) → review+
I have landed the patch as-is, it seemed to make intuitive sense to me for a superuser to have an override (even if they wouldn't use the UI).

We probably should do a unit test, but that can wait for another day. :) Leaving this bug open though.
Landed yesterday as:
https://github.com/mozilla/treeherder/commit/b57688d7747c763042df8c0ed6a60dff973936db

More context for any bounty consideration:
* The API credentials functionality was added in bug 1160111.
* Any logged in Treeherder user (we use Persona for login, so this is anyone with an email address) could delete the API token of any other user (but not view), ie perform a denial of service attack.
* Treeherder/Taskcluster no longer use API tokens to submit data, so wouldn't be affected (so any malicious usage wouldn't be tree closing).
* Autophone/mozmill-ci do use API tokens to submit data, so could have been blocked from submitting jobs (they account for <5% of job submissions).
* Most users reading from our API use unauthenticated requests, so wouldn't have been affected.
Assignee: emorley → wlachance
Blocks: 1160111
Group: webtools-security
Status: ASSIGNED → RESOLVED
Last Resolved: 2 years ago
Resolution: --- → FIXED
Summary: Possible IDOR Vulnerability within - https://treeherder.mozilla.org/credentials/ → Any logged in user can delete another user's API tokens
(Reporter)

Comment 13

2 years ago
Thanks for the quick turn around time on this. It is much appreciated.

Updated

2 years ago
Blocks: 1304658
Thank you for reporting! :-)
Flags: sec-bounty? → sec-bounty+
You need to log in before you can comment on or make changes to this bug.