802026 - (CVE-2013-0759) URL spoofing with credentials info of URL & 204

Status: VERIFIED FIXED

(Firefox :: Address Bar, defect)

Description

[Masato Kinugawa]

Attachment: 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]]

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]]

(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][[e-mail]]]

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][[e-mail]]]

Daniel, you moved this from LocationBar component to DocumentNavigation. Any reason why?

Comment 7

[:Gavin Sharp [email: [e-mail]]]

Attachment: 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: [e-mail]]]

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: [e-mail]]]

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

[Al Billings [:abillings - ex-MoCo]]

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]]

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: [e-mail]]]

Attachment: 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: [e-mail]]]

Attachment: esr10 patch

[Approval Request Comment]
(see comment 12)

Comment 14

[bhavana bajaj [:bajaj]]

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: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-beta/rev/421972d1929c

Comment 16

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/integration/mozilla-inbound/rev/83e7913cba52

Comment 17

[:Gavin Sharp [email: [e-mail]]]

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: [e-mail]]]

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: [e-mail]]]

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: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/de3c29abed65

Comment 21

[:Gavin Sharp [email: [e-mail]]]

Attachment: 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: [e-mail]]]

Attachment: ESR17 patch

For ESR17, I just added whenNewWindowLoaded to head.js.

Comment 23

[:Gavin Sharp [email: [e-mail]]]

Attachment: 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: [e-mail]]]

(In reply to :Gavin Sharp (use [e-mail] 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 25

[:Gavin Sharp [email: [e-mail]]]

https://hg.mozilla.org/releases/mozilla-esr10/rev/a68f770eb796
https://hg.mozilla.org/releases/mozilla-esr17/rev/f236b3b75eba

Comment 26

[:Gavin Sharp [email: [e-mail]]]

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]]

This is still broken on Aurora.

Comment 28

[:Gavin Sharp [email: [e-mail]]]

Oops, somehow managed to forget to open an Aurora TBPL tab :( Too many branches.
https://hg.mozilla.org/releases/mozilla-aurora/rev/640b594b297f

Comment 29

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-b2g18/rev/421972d1929c
https://hg.mozilla.org/releases/mozilla-b2g18/rev/ec6bb025e964
https://hg.mozilla.org/releases/mozilla-b2g18/rev/ef57a0236dbd

Comment 30

[Ed Morley [:emorley]]

https://hg.mozilla.org/mozilla-central/rev/83e7913cba52
https://hg.mozilla.org/mozilla-central/rev/84896936f0d4

Comment 31

[u279076]

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

[xisigr]

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>