Closed Bug 1303928 Opened 8 years ago Closed 8 years ago

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

Categories

(Tree Management :: Treeherder: API, defect, P1)

defect

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: griffin.francis.1993, Assigned: wlach)

References

Details

(Keywords: sec-high, wsec-authorization)

Attachments

(6 files)

Attached image 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.
Attached image Example Two.PNG
Attached image Example Three.PNG
I have confirmed this on Allizom with permission of Jonathan. Attached are more screenshots.
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?
Product: Websites → Release Engineering
QA Contact: hwine
Group: websites-security → webtools-security
Component: Tools → Treeherder: API
Priority: -- → P1
Product: Release Engineering → Tree Management
QA Contact: hwine
Version: unspecified → ---
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/
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
Attached patch PatchSplinter Review
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
Closed: 8 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
Thanks for the quick turn around time on this. It is much appreciated.
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.

Attachment

General

Created:
Updated:
Size: