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)
Tree Management
Treeherder: API
Tracking
(Not tracked)
RESOLVED
FIXED
People
(Reporter: griffin.francis.1993, Assigned: wlach)
References
Details
(Keywords: sec-high, wsec-authorization)
Attachments
(6 files)
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•8 years ago
|
||
Reporter | ||
Comment 2•8 years ago
|
||
Reporter | ||
Comment 3•8 years ago
|
||
I have confirmed this on Allizom with permission of Jonathan. Attached are more screenshots.
Reporter | ||
Comment 4•8 years ago
|
||
Reporter | ||
Comment 5•8 years ago
|
||
Comment 6•8 years ago
|
||
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•8 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•8 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•8 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
Assignee | ||
Comment 9•8 years ago
|
||
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 10•8 years ago
|
||
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+
Assignee | ||
Comment 11•8 years ago
|
||
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.
Comment 12•8 years ago
|
||
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
Reporter | ||
Comment 13•8 years ago
|
||
Thanks for the quick turn around time on this. It is much appreciated.
Comment 14•8 years ago
|
||
Thank you for reporting! :-)
Updated•8 years ago
|
Flags: sec-bounty? → sec-bounty+
You need to log in
before you can comment on or make changes to this bug.
Description
•