Last Comment Bug 802026 - (CVE-2013-0759) URL spoofing with credentials info of URL & 204
(CVE-2013-0759)
: URL spoofing with credentials info of URL & 204
Status: VERIFIED FIXED
[adv-main18+][adv-esr17+][adv-esr10+]
: csectype-spoof, sec-high
Product: Firefox
Classification: Client Software
Component: Location Bar (show other bugs)
: 16 Branch
: All All
: -- normal (vote)
: Firefox 20
Assigned To: :Gavin Sharp [email: gavin@gavinsharp.com]
: Anthony Hughes (:ashughes) [GFX][QA][Mentor]
Mentors:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-16 00:12 PDT by Masato Kinugawa
Modified: 2014-07-24 14:38 PDT (History)
10 users (show)
rforbes: sec‑bounty+
anthony.s.hughes: in‑testsuite?
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
wontfix
+
verified
+
verified
+
verified
18+
verified
18+
verified
fixed


Attachments
spoof_credentials_and_204.html (1.44 KB, text/html;charset=utf-8)
2012-10-16 00:12 PDT, Masato Kinugawa
no flags Details
patch (7.76 KB, patch)
2012-12-12 01:21 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
dao+bmo: review+
dveditz: sec‑approval+
Details | Diff | Review
beta/aurora patch (7.02 KB, patch)
2012-12-13 17:33 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
bajaj.bhavana: approval‑mozilla‑aurora+
bajaj.bhavana: approval‑mozilla‑beta+
bajaj.bhavana: approval‑mozilla‑esr17+
Details | Diff | Review
esr10 patch (7.07 KB, patch)
2012-12-13 17:34 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
bajaj.bhavana: approval‑mozilla‑esr10+
Details | Diff | Review
updated ESR10 patch (6.02 KB, patch)
2012-12-15 15:39 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
gavin.sharp: approval‑mozilla‑esr10+
Details | Diff | Review
ESR17 patch (7.70 KB, patch)
2012-12-15 15:40 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
gavin.sharp: approval‑mozilla‑esr17+
Details | Diff | Review
followup fix for trunk/beta (1.08 KB, patch)
2012-12-15 15:42 PST, :Gavin Sharp [email: gavin@gavinsharp.com]
no flags Details | Diff | Review

Description Masato Kinugawa 2012-10-16 00:12:54 PDT
Created attachment 671741 [details]
spoof_credentials_and_204.html

User Agent: Mozilla/5.0 (Windows NT 6.0; rv:16.0) Gecko/20100101 Firefox/16.0
Build ID: 20121010144125

Steps to reproduce:

1. Go to attachment file and click "go" button.
2. Address bar points "google.com". But in fact, it is not "google.com".



Expected results:

Firefox should display URL correctly.
Comment 3 Daniel Veditz [:dveditz] 2012-10-17 10:40:21 PDT
We've had problems with navigation, 204's and the address bar in the past. Is the old bug back or is this new?
Comment 4 Mark Goodwin [:mgoodwin] 2012-10-17 12:52:15 PDT
(In reply to Daniel Veditz [:dveditz] from comment #3)
> We've had problems with navigation, 204's and the address bar in the past.
> Is the old bug back or is this new?

I don't know if the UI redress issue is new; I also don't know if the technique for suppressing the warning dialog is known.
Comment 5 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-12-05 09:10:35 PST
So isn't this more like an UI issue. What I see is location bar which has 
http://www.google.com ...lots of white space... @l0.cm/fg.html
Comment 6 Olli Pettay [:smaug] (high review load, please consider other reviewers) 2012-12-05 09:11:48 PST
Daniel, you moved this from LocationBar component to DocumentNavigation. Any reason why?
Comment 7 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-12 01:21:37 PST
Created attachment 691245 [details] [diff] [review]
patch

There's a simpler way to reproduce:
1) Load "http://spoof             @gavinsharp.com" (copy/paste from here, those are some fancy unicode spaces).
2) (Accept the warning)
3) Focus URL bar, press Esc.

Expected: URL stays the same
Actual: URL switches to the "spoofy" value

The underlying issue is that we're not calling createExposableURI when URLBarSetURI reverts the URL to the current URI. This happens in the case above, but also when finishing toolbar customization and, since bug 724599, when a load fails, as in this bug's testcase.

I searched the addons MXR for users of XULBrowserWindow._uriFixup - edilee's addon is the only one I found find:
https://mxr.mozilla.org/addons/source/310123/bootstrap.js#1236
Comment 8 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-12 01:23:34 PST
Mardak: see the last part of comment 7. You'll need to stop using XULBrowserWindow._uriFixup in AwesomeBar HD. I recommend just retrieving the service  yourself rather than using a random Firefox property.
Comment 9 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-12 12:12:33 PST
Comment on attachment 691245 [details] [diff] [review]
patch

[Security approval request comment]
How easily can the security issue be deduced from the patch?

Fairly easily, given the test. It's clear that we're touching spoofing-related functionality (display of the URL in the URL bar, stripping of the user/pass).

Do comments in the patch, the check-in comment, or tests included in the patch paint a bulls-eye on the security problem?

It's clear what we're fixing, but probably not that clear how you would exploit this in practice.

Which older supported branches are affected by this flaw?

All.

If not all supported branches, which bug introduced the flaw?

N/A

Do you have backports for the affected branches? If not, how different, hard to create, and risky will they be?

Patch applies pretty much as-is to beta/aurora, very little uplift risk.

How likely is this patch to cause regressions; how much testing does it need?

I think it's unlikely to cause regressions - we're just being more consistent and applying existing URL bar display behavior to more cases. For beta/aurora, I can avoid the property removal that could potentially affect add-ons.
Comment 10 [On PTO until 6/29] 2012-12-12 14:10:31 PST
Adding Alex so we can discuss when we want to take this given that it affects branches and we're in the middle of the cycle.
Comment 11 Daniel Veditz [:dveditz] 2012-12-13 16:16:41 PST
Comment on attachment 691245 [details] [diff] [review]
patch

sec-approval+

For the branches--especially ESRs--we need a patch without the _uriFixup removal.
Comment 12 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-13 17:33:26 PST
Created attachment 692103 [details] [diff] [review]
beta/aurora patch

[Approval Request Comment]
Bug caused by (feature/regressing bug #): n/a
User impact if declined: URL bar vulnerable to spoofing
Testing completed (on m-c, etc.): none yet
Risk to taking this patch (and alternatives if risky): relatively low risk. No add-on impact, and we're just making existing code used for displaying URLs in the URL bar apply more broadly.
String or UUID changes made by this patch: none
Comment 13 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-13 17:34:08 PST
Created attachment 692106 [details] [diff] [review]
esr10 patch

[Approval Request Comment]
(see comment 12)
Comment 14 bhavana bajaj [:bajaj] 2012-12-14 16:06:43 PST
Comment on attachment 692103 [details] [diff] [review]
beta/aurora patch

Approving on branches as its a low risk patch for a sec high bug and unlikely to cause regressions.
Comment 15 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 12:52:32 PST
https://hg.mozilla.org/releases/mozilla-beta/rev/421972d1929c
Comment 16 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 12:59:22 PST
https://hg.mozilla.org/integration/mozilla-inbound/rev/83e7913cba52
Comment 17 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 13:00:45 PST
Comment on attachment 692106 [details] [diff] [review]
esr10 patch

This patch is going to need an update, the test won't work as-is on ESR10.
Comment 18 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 13:05:43 PST
Comment on attachment 692103 [details] [diff] [review]
beta/aurora patch

This patch will also need a test update for ESR17. Notes to self:
- whenNewWindowLoaded doesn't exist on the ESRs
- whenDelayedStartupFinished doesn't exist in the right head.js on ESR10
- openToolbarCustomizationUI/closeToolbarCustomizationUI don't exist on ESR10
- pageloaderror may not be a valid test on esr10 (since it's pre-bug 724599)
Comment 19 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 15:22:08 PST
Followup to fix some test bustage:
http://hg.mozilla.org/integration/mozilla-inbound/rev/84896936f0d4
https://hg.mozilla.org/releases/mozilla-beta/rev/ec6bb025e964
Comment 20 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 15:24:06 PST
https://hg.mozilla.org/releases/mozilla-aurora/rev/de3c29abed65
Comment 21 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 15:39:25 PST
Created attachment 692664 [details] [diff] [review]
updated ESR10 patch

I just removed the two non-working sub-tests ("customization" and "loading a new page)", leaving just the handleRevert test for ESR10.
Comment 22 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 15:40:07 PST
Created attachment 692665 [details] [diff] [review]
ESR17 patch

For ESR17, I just added whenNewWindowLoaded to head.js.
Comment 23 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 15:42:23 PST
Created attachment 692666 [details] [diff] [review]
followup fix for trunk/beta

This is the followup I landed in comment 19 to work around bug 822056 (which I didn't catch locally, because I was testing before per-window PB got turned on).
Comment 24 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 15:43:09 PST
(In reply to :Gavin Sharp (use gavin@gavinsharp.com for email) from comment #23)
> Created attachment 692666 [details] [diff] [review]
> followup fix for trunk/beta

(I also rolled this in when I pushed the original patch to Aurora)
Comment 26 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-15 15:59:17 PST
Sigh, the beta bustage was the same as the ESR bustage - whenNewWindowLoaded was not defined. I added it as with ESR-17:
https://hg.mozilla.org/releases/mozilla-beta/rev/ef57a0236dbd
Comment 27 Ryan VanderMeulen [:RyanVM] 2012-12-16 04:52:35 PST
This is still broken on Aurora.
Comment 28 :Gavin Sharp [email: gavin@gavinsharp.com] 2012-12-16 10:36:40 PST
Oops, somehow managed to forget to open an Aurora TBPL tab :( Too many branches.
https://hg.mozilla.org/releases/mozilla-aurora/rev/640b594b297f
Comment 31 Anthony Hughes (:ashughes) [GFX][QA][Mentor] 2013-01-04 11:53:16 PST
Confirmed reproducible with Firefox 16.0.

Verified fixed with:
* Firefox 20.0a1 2013-01-04
* Firefox 19.0a2 2013-01-04
* Firefox 18.0b7
* Firefox 10.0.12esr build1
* Firefox 17.0.1esrpre 2013-01-03

Is this something we can put in testsuite?
Comment 32 xu shaopei 2013-02-17 23:36:16 PST
Comment on attachment 671741 [details]
spoof_credentials_and_204.html

><!DOCTYPE html>
><html>
><head>
><meta charset="utf-8">
></head>
><body>
>222
><script>
>function a(){
>	window.open('http://www.google.com                                                                                                                                                                                    @l0.cm/fg.html', 'x');
>	setTimeout(function(){
>		window.open('http://l0.cm/204', 'x');
>	}, 500);
>}
></script>
><input type="button" value="go" onclick="a()">
></body>
></html>

Note You need to log in before you can comment on or make changes to this bug.