Project: chromium Issues People Development process History Sign in
New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.
Starred by 2 users
Status: Fixed
Owner:
Closed: Apr 27
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 2
Type: Bug-Security
Team-Security-UX



Sign in to add a comment
Security: RTL character in URL flips domain and path (Android 4.2 and earlier)
Reported by rayyan...@gmail.com, Apr 7 Back to list
Chrome Version: [56] + [stable] (Updated on April 3, 2017)
Operating System: [Android 4.1.2] 


Impact and Risk: the URL bar is the only reliable security indicator in browsers and if the only reliable security indicator could be controlled by an attacker it could carry adverse affects, For instance potentially tricking users into supplying sensitive information to a malicious website due to the fact that it could easily lead the users to believe that they are visiting is legitimate website as the address bar points to the correct website.

Reproduction Instructions/Proof of Concept:

1) Post the following link in the status bar: 127.0.0.1/ا/http://attack.com


2) You would notice that the URL has been flipped from Right to left and the status bar dispays http://attack.com/‭ا/127.0.0.1 while it displays the content from the IP address.

Suggested Fix: This bug does not work on iOS devices, hence, the screenshot was taken from an iOS device of google chrome which shows that URL must be shown like this.
 
PoC.jpg
17.7 KB View Download
Suggested Fixture.jpg
24.1 KB View Download
This appears to be the same repro as Issue 708981 ?
Comment 2 Deleted
Comment 3 Deleted
Comment 4 Deleted
Components: UI>Browser>Omnibox
Labels: Security_Severity-Low Security_Impact-Stable OS-Android
Owner: mgiuca@chromium.org
Status: Assigned
+mgiuca to dedupe.
Comment 6 Deleted
Project Member Comment 7 by sheriffbot@chromium.org, Apr 10
Labels: Pri-2
Hi, as with Issue 708981 I cannot reproduce this on Android in the latest version.

This is the same bug that was originally reported and fixed with  Issue 609680  on Android. However, I am not duping to that yet, because if this is in fact still an issue on M56 then it is a regression.

Admittedly, I don't have Chrome 56 here (testing on 59) but it is not an issue for me that I can see. It is working as intended.

Please confirm that you are testing on Chrome 56 for Android, and not an older build.

I'm also confused because here you say it is fixed on iOS, but in Issue 708981, you reported this exact same bug on iOS.
Hi, I just checked on Android again.. The bug is now not working on the Android 6.0.. However, It's working in older versions of Android (Latest version of chrome). To be more specific, Android version 4.1.2 - therefore, please look into the older versions of android such as 4.5 or 4.6 to check it. Regarding the iOS, I've sent the details, please look into it. Moreover, I apologize for the confusion I made. Thanks!
Summary: Security: RTL character in URL flips domain and path (Android) (was: Security: Adress Bar Spoofing)
Can't reproduce on Android 5.0 (Lollipop).

I am hunting around for an Android 4.x device. And not really sure how the Android version could affect this UX.
Cc: tedc...@chromium.org
+Ted: I'm having a hard time getting the right version of Android + the right version of Chrome. Do you have test devices? Are you able to repro this or find someone to do it?

The specific condition is Android 4.x + Chrome 56 (or any current channel of Chrome will do).
4.1 had a very broken implementation of RTL.  4.3 was really the first jellybean that was close to working.

All of the APIs that allow us to specify text direction and alignment are API 17.  4.1.2 is API 16.  I doubt there is anything that can be done here aside from disabling any RTL characters from being displayed (converting them all to punycode).
Really? :| Do you think we should do that? Should I get a PM from the Omnibox team to help decide here?
I'm honestly not super convinced we should, but that is more from a maintenance perspective for devices that are nearly 5 years old.  Our coverage for these old devices in minimal and any special logic we introduce is likely to be bit-rotting immediately.

I think it would be fine to bring this up with the omnibox team as an issue (and if disabling RTL characters is easy then I'm less hesitant to make the shift), but I am tempted to mark this as WontFix as this is a problem with Android and there is only so much we should fight that.
Cc: mgiuca@chromium.org
Owner: jdonnelly@chromium.org
OK so I'm not sure if this is working on Kit Kat. If it's a a <=Jellybean only problem then it affects about 1/8 of users according to https://developer.android.com/about/dashboards/index.html. If it also includes Kit Kat then it affects about 3/8 of users.

Adding jdonnelly for Omnibox (not sure if you are familiar with the Android code). See #15: if you think it's easy to just disable RTL character rendering in the Omnibox (I guess just blacklist them and make them punycode on Android JB or earlier) then could you please find someone to do that? Else, WontFix.

Thanks
In my opinion, Do check it out on 4.4 - Because according to some sites in 2015, there were 1.4 Billion active users using Android and in 2017, The number is obviously increased and 3/8 users is also counted as a big population. 
Cc: emilyschechter@chromium.org jdonnelly@chromium.org
Owner: tommycli@chromium.org
tommycli: I have a device we can flash KitKat on to test this.

tedchoc: can you confirm that if this issue is present on KitKat that 3/8 of all Android users would be affected? If so, this seems worth adding mgiuca's proposed fix.

emilyschechter: does that assessment sound right to you?
I'm basing the 1/8 and 3/8 metrics on this dashboard:
https://developer.android.com/about/dashboards/index.html

The precise figure is 11.9% on JellyBean and earlier; 31.9% on KitKat and earlier (as at April 3, 2017).
I just tested this on 4.4, and it looks fine.

I will proceed to work backwards. I'm assuming based on #13 tedchoc@, that this problem only affects 4.2 and before.

Screenshot_2017-04-21-19-04-50.png
62.6 KB View Download
4.3 also looks good.
Screenshot_2017-04-21-12-26-19.png
64.1 KB View Download
I can confirm that this affects Android 4.2 and before (API level 17 and before).

I think this is worth a fix since 10.8% of our users are at this version and before. That's still over 100M devices...

I'm investigating how we can fix it. I think there can be a small fix.
Summary: Security: RTL character in URL flips domain and path (Android 4.2 and earlier) (was: Security: RTL character in URL flips domain and path (Android))
Maybe it's worth just banning RTL characters from appearing in URLs in Android 4.2 and earlier?

The code to manage URL escaping is in net/base/escape.cc (which is very general code, it might be hard to put Android-specific logic in there).
mgiuca: Thanks for the pointer! I am looking into this today. :)
One proposed fix. See screenshots for before and after: https://codereview.chromium.org/2839843002/
Screenshot_2017-04-24-13-17-05.png
54.4 KB View Download
Screenshot_2017-04-24-15-59-51.png
55.0 KB View Download
New fix in PS2
Screenshot_2017-04-26-18-31-23.png
72.4 KB View Download
Project Member Comment 27 by bugdroid1@chromium.org, Apr 27
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e1fa8fc5972327ce78225c7de95153c63d3e3bb3

commit e1fa8fc5972327ce78225c7de95153c63d3e3bb3
Author: tommycli <tommycli@chromium.org>
Date: Thu Apr 27 19:06:56 2017

Android URL formatting: Disable RTL URLs for Android 4.2 and below.

Android 4.2 and below has a problematic RTL implementation. Force
formatted URLs to display in LTR.

BUG= 709417 

Review-Url: https://codereview.chromium.org/2839843002
Cr-Commit-Position: refs/heads/master@{#467740}

[modify] https://crrev.com/e1fa8fc5972327ce78225c7de95153c63d3e3bb3/chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java

Status: Fixed
Does it qualify for a bounty?
Cc: awhalley@chromium.org
#29: I think it falls into the bounty category, but the decision is not up to me (+awhalley).

The panel should keep in mind that this is a bug we already knew about, and had fixed ( Issue 609680 ). But we didn't know the previous fix was ineffective on older versions of Android.
If that's the case with android maybe that'd be the case with other operating systems such as iOS? Maybe tedc...@chromium.org have the idea of it since he was the one who told the reason in #15 for Android. (Still I'll check on other OS If get an access to suitable machine)
The fix on iOS was independent to the Android one, so it would be a big coincidence if it also had an issue with older versions of iOS. This is an Android-specific problem.
Project Member Comment 33 by sheriffbot@chromium.org, Apr 28
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Labels: reward-topanel
Let's see what the VRP panel say
Yes, FWIW, I think this report is worthy.
Fix should show up in Android Canary versions 3084.0 and later (not released yet).
Components: UI>Security>UrlFormatting UI>Internationalization>RTL
Labels: -Security_Severity-Low Security_Severity-High
Nice! I fixed a "High" security bug!
Labels: -Security_Severity-High Security_Severity-Medium
Downgrading this to Medium (fits the description of "Medium" spoof on https://www.chromium.org/developers/severity-guidelines, and also this is the exact same bug as  Issue 609680 , which is Medium, but on a much smaller set of devices).
Labels: -Security_Severity-Medium Security_Severity-High
Hi mgiuca@ - I'm reverting it to high, but that's for spotting that  Issue 609680  was still Medium. We discussed this at the VRP panel and agreed both fall into "Complete control over the apparent origin in the omnibox" which is high. Good point about the reduced population, but we don't like calling that a mitigation if we still support those platforms.  Cheers!
Labels: M-60
Labels: Merge-Review-59
Comment 44 Deleted
Comment 45 Deleted
Labels: -reward-topanel reward-unpaid reward-3000
Congratulations rayyanh12@! The VRP panel decided to award $3,000 for this bug.  A member of our finance team will be in touch shortly to arrange payment.  Also how would you like to be credited?

*** Boilerplate reminders! ***
Please do NOT publicly disclose details until a fix has been released to all our users. Early public disclosure may cancel the provisional reward. Also, please be considerate about disclosure when the bug affects a core library that may be used by other products. Please do NOT share this information with third parties who are not directly involved in fixing the bug. Doing so may cancel the provisional reward. Please be honest if you have already disclosed anything publicly or to third parties. Lastly, we understand that some of you are not interested in money. We offer the option to donate your reward to an established charity. If you prefer this option, let us know and we will also match your donation - subject to our discretion. Any rewards that are unclaimed after 12 months will be donated to a charity of our choosing.
*********************************


Labels: -reward-unpaid reward-inprocess
Comment 49 Deleted
#47: Thank you. Can we discuss that via email? 
rayyanh12@ - certainly - drop me a line at awhalley@chromium.org
awhalley: should this have Merge-Request-59?
Labels: -Merge-Review-59 Merge-Request-59
Yep - been in dev a good few days.  Thanks!
Project Member Comment 54 by sheriffbot@chromium.org, May 9
Labels: -Merge-Request-59 Hotlist-Merge-Approved Merge-Approved-59
Your change meets the bar and is auto-approved for M59. Please go ahead and merge the CL to branch 3071 manually. Please contact milestone owner if you have questions.
Owners: amineer@(Android), cmasso@(iOS), gkihumba@(ChromeOS), Abdul Syed@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member Comment 55 by bugdroid1@chromium.org, May 9
Labels: -merge-approved-59 merge-merged-3071
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/0c5c382d741cade4d1b89295a45a7fb4c97fe317

commit 0c5c382d741cade4d1b89295a45a7fb4c97fe317
Author: Tommy C. Li <tommycli@chromium.org>
Date: Tue May 09 16:34:10 2017

Android URL formatting: Disable RTL URLs for Android 4.2 and below.

Android 4.2 and below has a problematic RTL implementation. Force
formatted URLs to display in LTR.

BUG= 709417 

Review-Url: https://codereview.chromium.org/2839843002
Cr-Commit-Position: refs/heads/master@{#467740}
(cherry picked from commit e1fa8fc5972327ce78225c7de95153c63d3e3bb3)

Review-Url: https://codereview.chromium.org/2870943002 .
Cr-Commit-Position: refs/branch-heads/3071@{#480}
Cr-Branched-From: a106f0abbf69dad349d4aaf4bcc4f5d376dd2377-refs/heads/master@{#464641}

[modify] https://crrev.com/0c5c382d741cade4d1b89295a45a7fb4c97fe317/chrome/android/java/src/org/chromium/chrome/browser/omnibox/UrlBar.java

I just merged the patch to 59. Thanks!
Labels: -Hotlist-Merge-Approved
Labels: M-59
Labels: Release-0-M59
Labels: CVE-2017-5072
Cc: cjgrant@chromium.org
Project Member Comment 62 by sheriffbot@chromium.org, Aug 4
Labels: -Restrict-View-SecurityNotify allpublic
This bug has been closed for more than 14 weeks. Removing security view restrictions.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Sign in to add a comment