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 1 user

Issue metadata

Status: Fixed
Owner:
Closed: Jun 2014
Cc:
Components:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Security



Sign in to add a comment
link

Issue 352083: Security: Chrome for Android - URL bar spoof

Reported by khaga19...@gmail.com, Mar 13 2014

Issue description

VULNERABILITY DETAILS
This allows for phishing attacks where a malicious site can spoof the URL of another site.

VERSION
Chrome Version: 33.0.1750.136 stable
Operating System: Android 4.4.2; Nexus 5 Build/KOT49H
 
repro1.html
234 bytes View Download
repro2.html
297 bytes View Download

Comment 1 by jsc...@chromium.org, Mar 13 2014

Cc: nasko@chromium.org creis@chromium.org
Labels: Security_Impact-Beta OS-Android Cr-UI-Browser-Navigation Security_Impact-Stable Security_Severity-High
Owner: palmer@chromium.org
Status: Assigned
This looks like some bugs we used to have on other platforms, where the omnibox would get updated too early during a navigation. Adding some navigation knowledgeable people for assistance.

@palmer - Could you please route this to the correct owner on Android?

Comment 2 by ClusterFuzz, Mar 13 2014

Project Member
Labels: M-33 Pri-1

Comment 3 by ClusterFuzz, Mar 24 2014

Project Member
Labels: Nag
palmer@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz

Comment 4 by ClusterFuzz, Mar 31 2014

Project Member
Labels: -M-33 M-34

Comment 5 by ClusterFuzz, Apr 1 2014

Project Member
palmer@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz

Comment 6 by palmer@chromium.org, Apr 1 2014

Cc: nyquist@chromium.org klo...@chromium.org
Owner: tedc...@chromium.org
Ted, are you a good person for this? Or do you know someone who is? Thanks!

Comment 7 by creis@chromium.org, Apr 4 2014

Cc: dtrainor@chromium.org
@dtrainor: This repro makes me think it's similar to  issue 324969 , where we needed to make Android react to DidAccessInitialDocument.  Do you think that's involved here?

Comment 8 by dtrainor@chromium.org, Apr 7 2014

It does look like it's related... the fix was more to keep the Java Tab object from using the old incorrect URL in that case.  Maybe we're missing something from native here, although it looks like we're calling WebContents::GetURL(), which gets the visible entry from the navigation controller and returns the virtual url.

Comment 9 by ClusterFuzz, Apr 10 2014

Project Member
tedchoc@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz

Comment 10 by ClusterFuzz, Apr 18 2014

Project Member
tedchoc@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz

Comment 11 by ClusterFuzz, Apr 26 2014

Project Member
tedchoc@: Uh oh! This issue is still open and hasn't been updated in the last 7 days. Since this is a serious security vulnerability, we want to make sure progress is happening. Can you update the bug with current status, and what, if anything, is blocking?

If you are not the right Owner for this bug, please find someone else to own it as soon as possible and remove yourself as Owner.

If the issue is already fixed or you are to unable to reproduce it, please close the bug. (And thanks for fixing the bug!).

These nags can be disabled by adding a 'WIP' label and an optional codereview link.

- Your friendly ClusterFuzz

Comment 12 by kenrb@chromium.org, Apr 28 2014

Labels: -Nag
Owner: dtrainor@chromium.org
dtrainor: Do you think you could have a look at this one?

Comment 13 by dtrainor@chromium.org, May 2 2014

Will take a look.

Comment 14 by dtrainor@chromium.org, May 6 2014

What's the expected behavior here?  The navigation controller entries all return keitahaga.com (active, visible, and pending).  The URL and the virtual URL also all match.  Desktop shows about:blank.  What should we be querying to get the right URL?

Comment 15 by creis@chromium.org, May 6 2014

Desktop is using NavigationControllerImpl::GetVisibleEntry(), so if it's showing about:blank that's where it's coming from.  That takes into account things like whether it's the initial navigation (false for document.write) and whether the initial document has been accessed.

I'm surprised you're seeing the attack URL from GetVisibleEntry().  Can you check how that code's actual behavior differs between desktop and Android, and why they're returning different results?

Comment 16 by dtrainor@chromium.org, May 7 2014

Still digging some more... looks like the following is happening (still haven't tracked down the trigger though):

1. We have a pending entry with is_renderer_initiated = true AND IsUnmodifiedBlankTab.  This properly shows about:blank
2. At some point we get a FrameHostMsg_OpenURL from the renderer (for the new tab I guess?)
3. This builds a new pending entry that has is_renderer_initiated = false.  So even when IsUnmodifiedBlankTab is false we still show this pending entry as the valid url.

I need to track why we're getting an OpenURL message.

Comment 17 by dtrainor@chromium.org, May 7 2014

Ah I think I tracked it down.  We're losing data in the jni bridge from native -> java -> native... We don't pass is_renderer_initiated so that gets lost in the translation process.  Hmm it looks like we might lose other data in this transfer.  I don't know if all of it is required though.  I can add the is_renderer_initiated though.

Comment 18 by dtrainor@chromium.org, May 7 2014

Ok actually tracked down the cause.  It wasn't JNI related but it it happens in web_contents_delegate_android.  We just call web_contents->GetController().LoadURL(...) and pass in a few parameters.  We should really be calling web_contents->GetController().LoadURLWithParams(LoadURLParams&) to pass the relevant data.  We're different here because we don't use browser_navigator (we don't have a browser) and don't use chrome::Navigate to do our navigations.  There is a *ton* of stuff in Navigate that we're probably missing/rewriting throughout the codebase... we should look at refactoring that to maybe not rely on a browser instance and sharing that to Android.

I have a fix that populates the LoadURLParams from an OpenURLParams and use that in the right place.  I will upload that shortly.

Comment 19 by ClusterFuzz, May 13 2014

Project Member
Labels: -M-34 Deadline-Exceeded M-35
You have far exceeded the 60-day deadline for fixing this high severity security vulnerability.

We commit ourselves to this deadline and appreciate your utmost priority on this issue.

If you are unable to look into this soon, please find someone else to own this.

- Your friendly ClusterFuzz

Comment 20 by infe...@chromium.org, May 13 2014

Dtrainor@, is this a WIP. Is there a codereview started (please add link here), if yes, please add WIP in the labels list.

Comment 21 by dtrainor@chromium.org, May 13 2014

Labels: WIP
Status: Started
https://chromiumcodereview.appspot.com/267253007/ and https://chrome-internal-review.googlesource.com/#/c/163332/ are the two related CLs.  Working on the best way to test the fix and it might have to be downstream until we have a proper concept of multiple tabs upstream.

Comment 22 by bugdroid1@chromium.org, May 30 2014

Project Member
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/98a50b76141f0b14f292f49ce376e6554142d5e2

commit 98a50b76141f0b14f292f49ce376e6554142d5e2
Author: dtrainor@chromium.org <dtrainor@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Fri May 30 17:44:30 2014

Use LoadURLWithParams in ChromeWebContentsDelegateAndroid

Build a LoadURLParams object from the OpenURLParams and properly set all
parameters on that object when calling into NavigationController.  This makes
sure we set the correct state for the load.

BUG= 352083 

Review URL: https://codereview.chromium.org/267253007

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@273865 0039d316-1c4b-4281-b951-d872f2087c98

Comment 23 by bugdroid1@chromium.org, May 30 2014

Project Member
------------------------------------------------------------------
r273865 | dtrainor@chromium.org | 2014-05-30T17:44:30.362656Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/android/content_view_core_impl.h?r1=273865&r2=273864&pathrev=273865
   M http://src.chromium.org/viewvc/chrome/trunk/src/android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java?r1=273865&r2=273864&pathrev=273865
   M http://src.chromium.org/viewvc/chrome/trunk/src/components/web_contents_delegate_android/web_contents_delegate_android.cc?r1=273865&r2=273864&pathrev=273865
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/android/java/src/org/chromium/chrome/browser/Tab.java?r1=273865&r2=273864&pathrev=273865
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java?r1=273865&r2=273864&pathrev=273865
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/android/tab_android.cc?r1=273865&r2=273864&pathrev=273865
   M http://src.chromium.org/viewvc/chrome/trunk/src/components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java?r1=273865&r2=273864&pathrev=273865
   M http://src.chromium.org/viewvc/chrome/trunk/src/chrome/browser/android/tab_android.h?r1=273865&r2=273864&pathrev=273865
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/public/android/java/src/org/chromium/content/browser/LoadUrlParams.java?r1=273865&r2=273864&pathrev=273865
   M http://src.chromium.org/viewvc/chrome/trunk/src/content/browser/android/content_view_core_impl.cc?r1=273865&r2=273864&pathrev=273865

Use LoadURLWithParams in ChromeWebContentsDelegateAndroid

Build a LoadURLParams object from the OpenURLParams and properly set all
parameters on that object when calling into NavigationController.  This makes
sure we set the correct state for the load.

BUG= 352083 

Review URL: https://codereview.chromium.org/267253007
-----------------------------------------------------------------

Comment 24 by bugdroid1@chromium.org, May 30 2014

Project Member
The following change refers to this bug:
https://chrome-internal-review.googlesource.com/163332

Comment 25 by dtrainor@chromium.org, May 30 2014

Labels: -WIP Merge-Requested M-36
Fixed on trunk.  Added M-36 label as well.  Merge requested for M-36 first.

Comment 26 by ClusterFuzz, May 31 2014

Project Member
Labels: ReleaseBlock-Stable

Comment 27 by matthewyuan@chromium.org, Jun 3 2014

Labels: -Merge-Requested Merge-Approved
Approved for 36.  Please re-request if you want it for 35.

Comment 28 by timwillis@chromium.org, Jun 3 2014

Labels: -M-35
Removing M-35 (this won't make that timeline).

dtrainor@ - please merge this to M-36 (branch 1985).

Comment 29 by dtrainor@chromium.org, Jun 4 2014

Labels: -Merge-Approved Merge-Merged
Status: Fixed

Comment 30 by bugdroid1@chromium.org, Jun 4 2014

Project Member
The following change refers to this bug:
https://chrome-internal-review.googlesource.com/165232

Comment 31 by bugdroid1@chromium.org, Jun 4 2014

Project Member
Labels: merge-merged-1985
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/086e4be496f864c0379507a03239caadffbc79c3

commit 086e4be496f864c0379507a03239caadffbc79c3
Author: dtrainor@chromium.org <dtrainor@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed Jun 04 19:57:19 2014

Merge 273865 "Use LoadURLWithParams in ChromeWebContentsDelegate..."

> Use LoadURLWithParams in ChromeWebContentsDelegateAndroid
> 
> Build a LoadURLParams object from the OpenURLParams and properly set all
> parameters on that object when calling into NavigationController.  This makes
> sure we set the correct state for the load.
> 
> BUG= 352083 
> 
> Review URL: https://codereview.chromium.org/267253007

TBR=dtrainor@chromium.org

Review URL: https://codereview.chromium.org/317733005

git-svn-id: svn://svn.chromium.org/chrome/branches/1985/src@274888 0039d316-1c4b-4281-b951-d872f2087c98

Comment 32 by bugdroid1@chromium.org, Jun 4 2014

Project Member
------------------------------------------------------------------
r274888 | dtrainor@chromium.org | 2014-06-04T19:57:19.618641Z

Changed paths:
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/content/public/android/java/src/org/chromium/content/browser/LoadUrlParams.java?r1=274888&r2=274887&pathrev=274888
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/content/browser/android/content_view_core_impl.cc?r1=274888&r2=274887&pathrev=274888
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/content/browser/android/content_view_core_impl.h?r1=274888&r2=274887&pathrev=274888
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/android_webview/java/src/org/chromium/android_webview/AwWebContentsDelegateAdapter.java?r1=274888&r2=274887&pathrev=274888
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/components/web_contents_delegate_android/web_contents_delegate_android.cc?r1=274888&r2=274887&pathrev=274888
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/chrome/android/java/src/org/chromium/chrome/browser/Tab.java?r1=274888&r2=274887&pathrev=274888
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java?r1=274888&r2=274887&pathrev=274888
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/chrome/browser/android/tab_android.cc?r1=274888&r2=274887&pathrev=274888
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/components/web_contents_delegate_android/android/java/src/org/chromium/components/web_contents_delegate_android/WebContentsDelegateAndroid.java?r1=274888&r2=274887&pathrev=274888
   M http://src.chromium.org/viewvc/chrome/branches/1985/src/chrome/browser/android/tab_android.h?r1=274888&r2=274887&pathrev=274888

Merge 273865 "Use LoadURLWithParams in ChromeWebContentsDelegate..."

> Use LoadURLWithParams in ChromeWebContentsDelegateAndroid
> 
> Build a LoadURLParams object from the OpenURLParams and properly set all
> parameters on that object when calling into NavigationController.  This makes
> sure we set the correct state for the load.
> 
> BUG= 352083 
> 
> Review URL: https://codereview.chromium.org/267253007

TBR=dtrainor@chromium.org

Review URL: https://codereview.chromium.org/317733005
-----------------------------------------------------------------

Comment 33 by ClusterFuzz, Jun 4 2014

Project Member
Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify

Comment 34 by bugdroid1@chromium.org, Jun 5 2014

Project Member
The following revision refers to this bug:
  https://chrome-internal.googlesource.com/bling/chromium.git/+/086e4be496f864c0379507a03239caadffbc79c3

commit 086e4be496f864c0379507a03239caadffbc79c3
Author: dtrainor@chromium.org <dtrainor@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98>
Date: Wed Jun 04 19:57:19 2014

Comment 35 by bugdroid1@chromium.org, Jun 10 2014

Project Member
The following change refers to this bug:
https://chrome-internal-review.googlesource.com/165553

Comment 36 by infe...@chromium.org, Jun 17 2014

Labels: reward-topanel

Comment 37 by timwillis@chromium.org, Jul 14 2014

Labels: Release-0-M36

Comment 38 by timwillis@chromium.org, Jul 14 2014

Labels: -reward-topanel reward-unpaid reward-3000
Congratulations- $3000 for this report! 

We'll credit you as "khaga19937" in our release notes. If you want to be known as another name, please update this bug ASAP with that name.

Someone should be in contact within 1-2 weeks to arrange payment. If you haven't received an update by then, please either update this bug or e-mail me directly.

*** 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.
          *********************************

Comment 39 by timwillis@chromium.org, Jul 14 2014

Cc: timwil...@gmail.com

Comment 40 by timwillis@chromium.org, Jul 15 2014

Labels: CVE-2014-3159

Comment 41 by khaga19...@gmail.com, Jul 15 2014

Credit as "Keita Haga" would be good. Thanks! :-)

Comment 43 by khaga19...@gmail.com, Jul 17 2014

Thanks!

Comment 44 by timwillis@chromium.org, Jul 22 2014

Labels: -reward-unpaid reward-inprocess

Comment 45 by ClusterFuzz, Sep 11 2014

Project Member
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.

Comment 46 by timwillis@chromium.org, Sep 17 2014

Labels: -reward-inprocess
Processing via our e-payment system can take a few weeks, but reward should be on its way to you. Thanks again for your help!

Comment 47 by ClusterFuzz, Feb 2 2016

Project Member
Labels: -Security_Impact-Beta

Comment 48 by sheriffbot@chromium.org, Oct 1 2016

Project Member
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

Comment 49 by sheriffbot@chromium.org, Oct 2 2016

Project Member
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

Comment 50 by mbarbe...@chromium.org, Oct 2 2016

Labels: allpublic

Comment 51 by awhalley@chromium.org, Apr 25 2018

Labels: CVE_description-submitted

Sign in to add a comment