Closed Bug 1254227 Opened 8 years ago Closed 8 years ago

MozReview auth delegation allows sending out phishing mails via Bugzilla

Categories

(bugzilla.mozilla.org Graveyard :: Extensions: MozReview Integration, defect)

Production
defect
Not set
normal

Tracking

(Not tracked)

RESOLVED FIXED

People

(Reporter: jwkbugzilla, Assigned: dylan)

References

Details

(Keywords: sec-moderate, wsec-injection)

Attachments

(1 file)

You can try it out by clicking this link: https://bugzilla.mozilla.org/auth.cgi?callback=https://reviewboard.mozilla.org/mozreview/bmo_auth_callback/&description=kabanga'%20has%20been%20created.%0A%0AIMPORTANT%3A%20If%20you%20didn't%20create%20this%20key%20pleases%20click%20http%3A%2F%2Fbugzilla-mozilla.org%2Ffoobar.cgi%20to%20change%20your%20password!%0A%0ASome%20additional%20boring%20info%20follows%2E%2E%2E

This will create a new API key for you and redirect you to reviewboard.mozilla.org, this can happen in background (via an image embedded into a site). You will get an email about the new API key which is normally a good thing, except that in this case it will read:

> [This e-mail has been automatically generated]
> 
> A new Bugzilla API key, with the
> description 'kabanga' has been created.
> 
> IMPORTANT: If you didn't create this key pleases click http://bugzilla-mozilla.org/foobar.cgi to change your password!
> 
> Some additional boring info follows...' has been created. You can view
> or update the key at the following URL:
> 
> https://bugzilla.mozilla.org/userprefs.cgi?tab=apikey
> 
> IMPORTANT: If you did not request a new key, your Bugzilla account
> ...

Note how the first "IMPORTANT" message is a fake directing you to a potential phishing site - injected via API key description. The second message is the real one, coming from Bugzilla.

I think that in this particular case the extension should override the description to say "MozReview" regardless of any query parameters. As far as the general Bugzilla functionality goes, it might be better to stop quoting the description in mails altogether - maybe the first letters of the API key will do as well.
Assignee: nobody → dylan
All legitimate users of API keys are currently using single words, like "mozreview" or "BZDeck".
Also, mozreview-ness should be using the app_id (which is a hash of the url and description.)

i really wish I had fought harder for the "app registry" concept for this feature.
Depends on: 1163761
Attached patch 1254227_1.patchSplinter Review
1) skipping consent requires a matching app id (which should have been done to begin with)
2) limit descriptions from auth delegation to only words
3) truncate descriptions to 10 chars in emails.

These are all mitigations. The right behavior remains to allow a registry as  discussed previously.
Attachment #8727634 - Flags: review?(dkl)
Flags: sec-bounty?
wsec-injection -- is that correct? wouldn't that refer to javascript or html injected into a page, which is not this particular bug?
Flags: needinfo?(amuntner)
Comment on attachment 8727634 [details] [diff] [review]
1254227_1.patch

Review of attachment 8727634 [details] [diff] [review]:
-----------------------------------------------------------------

r=dkl
Attachment #8727634 - Flags: review?(dkl) → review+
(In reply to Dylan William Hardison [:dylan] from comment #4)
> wsec-injection -- is that correct? wouldn't that refer to javascript or html
> injected into a page, which is not this particular bug?

From https://wiki.mozilla.org/WebAppSec/Web_App_Severity_Ratings:

> wsec-injection 	Injection attacks other than SQLi or XSS

Sounds about right. Also matches the definition under https://www.owasp.org/index.php/Top_10_2013-A1-Injection (important part is "untrusted data", not the target of the injection).
To ssh://gitolite3@git.mozilla.org/webtools/bmo/bugzilla.git
   ad2b169..9cc89d3  master -> master
Status: NEW → RESOLVED
Closed: 8 years ago
Flags: needinfo?(amuntner)
Resolution: --- → FIXED
Pushed
Group: bugzilla-security
Flags: sec-bounty? → sec-bounty+
Product: bugzilla.mozilla.org → bugzilla.mozilla.org Graveyard
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: