1284372 - (CVE-2016-5267) Firefox Mobile Address Bar Spoofing

Status: VERIFIED FIXED

(Firefox for Android Graveyard :: General, defect)

Description

[Rafay Baloch]

Attachment: spoofed1.png

User Agent: Mozilla/5.0 (Windows NT 6.1; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/51.0.2704.103 Safari/537.36
Firefox for Android

Steps to reproduce:

- Visit - https://jsfiddle.net/ktfegrhf/3/ using latest version of Firefox Mobile 
- Click on the link and observe the URL 


Actual results:

Due to mishandling of arabic RTL characters firefox for mobile causes the complete URL to be displayed from Right to left instead of left to right. 


Expected results:

The URL should be been displayed from LTR:

Expected Result:

http://عربي.امارات/google.com/test/test/test

Rendered Results

google.com/test/test/test/عربي.امارات


This is perfectly fine on firefox for mobile

Comment 1

[Rafay Baloch]

Attachment: spoof2.png

Comment 2

[Rafay Baloch]

Attachment: firefoxDesktopnotspoofed.png

Comment 3

[Rafay Baloch]

Correction: This is perfectly fine on firefox for mobile

To 

Correction: This is perfectly fine on Firefox Desktop Version.

Comment 4

[Kevin Brosnan [Ex-Mozilla]]

Confirmed this behavior. Using the jsfiddle you need to long press and select open in new tab. NI to Margaret for an assignee.

Comment 5

[:Margaret Leibovic]

Sebastian, can you look into this?

Comment 6

[Sebastian Kaspari (:sebastian; :pocmo)]

The same thing is happening in Chrome but as they are "scrolling" the URL (a variation of bug 1271998) and as their layout actually seems to support RTL it's not looking like a different domain.

I guess the correct solution would be to properly support RTL (at least in the toolbar): meta bug 702845.

In the short term we could try to force LTR. This will display the URL as expected in comment 0.

Comment 7

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: 1284372-force-LTR-URL.patch

Comment 8

[Sebastian Kaspari (:sebastian; :pocmo)]

Attachment: urlbar-forced-ltr.png

Comment 9

[Rafay Baloch]

Yes, LTR should be properly enforced and all URL's must always be displayed from Left-To-Right no matter what the direction of the content maybe, However, what needs to looked is if it won't cause any issues with appearance of RTL URL's.

Comment 10

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8768373 [details] [diff] [review]
1284372-force-LTR-URL.patch

@Grisha: Can you take a look? :)

Comment 11

[:Grisha Kruglov]

Comment on attachment 8768373 [details] [diff] [review]
1284372-force-LTR-URL.patch

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

This looks fine. I don't know much about unicode directionality stuff; but, since you're adding a public method to StringUtils for forcing LTR, I'll ask: would that approach (appending a special char) work generally, outside of this specific usecase? e.g. can I pass in an already LTR string into it, and safely use/render/edit/etc result afterwards? Perhaps that method should assert that its input is RTL.

Comment 12

[Sebastian Kaspari (:sebastian; :pocmo)]

https://hg.mozilla.org/integration/fx-team/rev/4f48e514c1ee8dbc7fea916f7b88659d20cfb655
Bug 1284372 - URL bar: Force LTR for the URL. r=grisha

Comment 13

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/4f48e514c1ee

Comment 14

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

Rafay, please don't set sec-bounty? on your bugs. The procedure is to email [e-mail] and ask to be considered for bounties. See https://www.mozilla.org/en-US/security/client-bug-bounty/ under "Claiming a bounty".

Comment 15

[Marcia Knous [:marcia]]

Tracking 50+ for this security fix.

Comment 16

[Sebastian Kaspari (:sebastian; :pocmo)]

Comment on attachment 8768373 [details] [diff] [review]
1284372-force-LTR-URL.patch

This should be fairly easy to uplift.

Approval Request Comment

[Feature/regressing bug #]: All versions are affected. This might have been the case since the beginning of time.

[User impact if declined]: See screenshot in comment 0. We display some parts of a specifically crafted URLs in an unexpected order. This can be used to make you think you are browsing a different domain than you actually are.

[Describe test coverage new/current, TreeHerder]: Local testing and this is now in Nightly since a couple of days.

[Risks and why]: Lowish. We prepend (RTL) URLs with a control character that forces the view to display it like a LTR string. I can't think of any side effects because at this point this string is only used for displaying (not editing) the URL.

[String/UUID change made/needed]: -

Comment 17

[Sylvestre Ledru [:Sylvestre]]

Comment on attachment 8768373 [details] [diff] [review]
1284372-force-LTR-URL.patch

OK, fix this issue. Taking it.
Should be in 48 beta 8 or 9.

Ioana, could you verify this? Thanks

Comment 18

[Carsten Book [:Tomcat]]

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

Comment 19

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/releases/mozilla-beta/rev/e93bcb91f4c7

Comment 20

[u549602]

Verified as fixed on all branches(48.0b9, Aurora 49.0a2/2016-07-18 and Nightly
50.0a1/2016-07-18) on a Samsung Galaxy S6 Edge (Android 6.0.1) and on a Samsung Galaxy Tab S2(Android 5.0.2)