Closed Bug 1284372 (CVE-2016-5267) Opened 4 years ago Closed 4 years ago
Firefox Mobile Address Bar Spoofing
32.77 KB, image/png
79.39 KB, image/png
16.98 KB, image/png
6.76 KB, patch
|Details | Diff | Splinter Review|
25.06 KB, image/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
Component: Untriaged → General
Product: Firefox → Firefox for Android
Version: unspecified → 50 Branch
Correction: This is perfectly fine on firefox for mobile To Correction: This is perfectly fine on Firefox Desktop Version.
Confirmed this behavior. Using the jsfiddle you need to long press and select open in new tab. NI to Margaret for an assignee.
Status: UNCONFIRMED → NEW
tracking-fennec: --- → ?
Ever confirmed: true
Sebastian, can you look into this?
Assignee: nobody → s.kaspari
Flags: needinfo?(margaret.leibovic) → needinfo?(s.kaspari)
Status: NEW → ASSIGNED
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.
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 on attachment 8768373 [details] [diff] [review] 1284372-force-LTR-URL.patch @Grisha: Can you take a look? :)
Attachment #8768373 - Flags: review?(margaret.leibovic) → review?(gkruglov)
tracking-fennec: ? → 48+
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.
Attachment #8768373 - Flags: review?(gkruglov) → review+
https://hg.mozilla.org/integration/fx-team/rev/4f48e514c1ee8dbc7fea916f7b88659d20cfb655 Bug 1284372 - URL bar: Force LTR for the URL. r=grisha
Rafay, please don't set sec-bounty? on your bugs. The procedure is to email email@example.com and ask to be considered for bounties. See https://www.mozilla.org/en-US/security/client-bug-bounty/ under "Claiming a bounty".
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 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
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)
You need to log in before you can comment on or make changes to this bug.