New issue
Advanced search Search tips
Note: Color blocks (like or ) mean that a user may not be available. Tooltip shows the reason.

Issue 780708 link

Starred by 1 user

Issue metadata

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

Blocking:
issue 780715



Sign in to add a comment

Security: "googlechrome" scheme allows opening downloaded files in content scheme

Project Member Reported by wfh@chromium.org, Nov 2 2017

Issue description

Ref: ZDI-CAN-5355

This bug was responsibly disclosed during pwn2own mobile 2017.

This bug is similar to  issue 659492 . The following text was provided by the reporter. It allows
files to be opened under the content scheme.

The file provider definition can be found in Chrome's manifest file, i.e. 'com.android.chrome/AndroidManifest.xml':

<provider android:authorities="com.android.chrome.FileProvider" android:exported="false"
 android:grantUriPermissions="true"
 android:name="org.chromium.chrome.browser.util.ChromeFileProvider">
  <meta-data android:name="android.support.FILE_PROVIDER_PATHS"
   android:resource="@xml/file_paths"/>
</provider>

The 'com.android.chrome/res/xml/file_paths.xml' specifies the folders accessible via this file
 provider.

<?xml version="1.0" encoding="utf-8"?>
 <paths xmlns:android="http://schemas.android.com/apk/res/android">
  <files-path name="images" path="images/" />
  <cache-path name="cache" path="net-export/" />
  <cache-path name="webapk" path="webapks/" />
  <external-path name="downloads" path="Download/" />
</paths>

The 'external-path' element represents file locations relative to the root of the SD card. This
means that 'Download/' translates to '/sdcard/Download/' which is exactly what we need.

The complete URI for the downloadme.html file, in the context of the Chrome file provider,
would be 'content://com.android.chrome.FileProvider/downloads/downloadme.html'. Last year,
an attacker would simply auto-click this link without any issues. However, Chrome patched
the 'http(s)' to 'content' transitions following our Mobile Pwn2Own 2016 submission.
Fortunately enough, they overlooked the 'googlechrome' scheme which effectively allows identical transitions. 

<activity-alias android:exported="true" android:name="com.google.android.apps.chrome.Main"
android:targetActivity="org.chromium.chrome.browser.document.ChromeLauncherActivity">
 [SNIP]
 <intent-filter>
 <action android:name="android.intent.action.VIEW"/>
 <category android:name="android.intent.category.DEFAULT"/>
 <category android:name="android.intent.category.BROWSABLE"/>
 <data android:scheme="googlechrome"/>   <---- HERE
 <data android:scheme="http"/>
 <data android:scheme="https"/>
 <data android:scheme="about"/>
 <data android:scheme="javascript"/>
 </intent-filter>
 [SNIP]
</activity-alias> 

This poorly documented scheme allows specifying a 'url' parameter which would be subsequently used
as target URI. Knowing this, we can leverage this problem by modifying our URI from
'content://com.android.chrome.FileProvider/downloads/downloadme.html' to
'googlechrome://navigate?url=content://com.android.chrome.FileProvider/downloads/down
loadme.html'.

This is where the second step takes place. We auto-click a link pointing to this URI which is where we
need the page to have been opened by a user gesture since this is a custom scheme. The auto-click
then triggers an intent processed by Chrome which extracts the 'url' parameter value and treats it as
the final URI. We can observe a transition from SBrowser to Chrome where Chrome ends up rendering
'content://com.android.chrome.FileProvider/downloads/downloadme.html'. 

 

Comment 1 by wfh@chromium.org, Nov 2 2017

hmm see also  issue 714442 
This was previously reported as  issue 714442  and we thought we'd fixed it, but it appears we didn't >_<

Comment 3 by wfh@chromium.org, Nov 2 2017

Cc: rsesek@chromium.org creis@chromium.org qin...@chromium.org nasko@chromium.org

Comment 4 by wfh@chromium.org, Nov 2 2017

Components: Mobile>Intents

Comment 5 by wfh@chromium.org, Nov 2 2017

Blocking: 780715

Comment 6 by wfh@chromium.org, Nov 2 2017

Cc: mariakho...@chromium.org

Comment 7 by vakh@chromium.org, Nov 2 2017

Owner: qin...@chromium.org
Status: Assigned (was: Unconfirmed)
qinmin -- since this is related to  issue 714442 , assigning to you for further triage.

Comment 8 by vakh@chromium.org, Nov 2 2017

Labels: Security_Severity-High
I am getting the same error as we are looking into  issue 714442   :

"Navigation is blocked: googlechrome://navigate?url=content://com.android.chrome.FileProvider/downloads/attack.html"

Any idea how to trigger the security vulnerability?




It feels like there is some finch experiment or something that disables navigation to googlechrome://navigate?url=content://com.android.chrome.FileProvider/downloads/attack.html.

I was able to repro the issue on my first run of M62 stable this morning, but now the issue is no longer reproducible, with both stable and dev build. Since finch doesn't work on the first run, so it probably explains why this isn't working after the first run.

It could be due to PlzNavigate, which is controlled through Finch in M62. You can explicitly disable it with --disable-browser-side-navigation and check. I don't expect it to be the root cause, but it will be very interesting to check if it is and understand why.
No, it is not PlzNavigate as I've disabled it in Chrome://flags.
Ok, keep in mind that it is the default navigation stack starting in M62 and old code will be removed likely in M65, so any testing we do must not disable it, as we won't be testing what is deployed to users.
I was able to reproduce in 58 and 61 using the following:

https://arw.me/f/9fa0c41a59f3fd9ba62f00282f83f8.html
Project Member

Comment 15 by bugdroid1@chromium.org, Nov 4 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/e29f7184a20c69e484adfd115b7dcdc4f774ef5b

commit e29f7184a20c69e484adfd115b7dcdc4f774ef5b
Author: Min Qin <qinmin@chromium.org>
Date: Sat Nov 04 04:50:25 2017

Only allow googlechrome protocol for http(s) urls

The protocol should only be used to navigate to http(s) urls, no content or
file uri.

BUG= 780708 

Change-Id: I00ea7e0fd31ec9e379ddcb5126265a6744de822d
Reviewed-on: https://chromium-review.googlesource.com/752202
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/heads/master@{#514030}
[modify] https://crrev.com/e29f7184a20c69e484adfd115b7dcdc4f774ef5b/chrome/android/java/src/org/chromium/chrome/browser/AppIndexingUtil.java
[modify] https://crrev.com/e29f7184a20c69e484adfd115b7dcdc4f774ef5b/chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java
[modify] https://crrev.com/e29f7184a20c69e484adfd115b7dcdc4f774ef5b/chrome/android/javatests/src/org/chromium/chrome/browser/IntentHandlerTest.java

Labels: Merge-Request-63 M-63
Status: Started (was: Assigned)
Project Member

Comment 17 by sheriffbot@chromium.org, Nov 4 2017

Labels: -Merge-Request-63 Merge-Review-63 Hotlist-Merge-Review
This bug requires manual review: M63 has already been promoted to the beta branch, so this requires manual review
Please contact the milestone owner if you have questions.
Owners: cmasso@(Android), cmasso@(iOS), gkihumba@(ChromeOS), govind@(Desktop)

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 18 by sheriffbot@chromium.org, Nov 4 2017

Labels: Security_Impact-Beta
Project Member

Comment 19 by sheriffbot@chromium.org, Nov 4 2017

Labels: ReleaseBlock-Stable
This is a serious security regression. If you are not able to fix this quickly, please revert the change that introduced it.

If this doesn't affect a release branch, or has not been properly classified for severity, please update the Security_Impact or Security_Severity labels, and remove the ReleaseBlock label. To disable this altogether, apply ReleaseBlock-NA.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 20 by sheriffbot@chromium.org, Nov 4 2017

Status: Fixed (was: Started)
Please mark security bugs as fixed as soon as the fix lands, and before requesting merges. This update is based on the merge- labels applied to this issue. Please reopen if this update was incorrect.

For more details visit https://www.chromium.org/issue-tracking/autotriage - Your friendly Sheriffbot
Project Member

Comment 21 by sheriffbot@chromium.org, Nov 5 2017

Labels: -Restrict-View-SecurityTeam Restrict-View-SecurityNotify
Please verify the fix in the latest canary.
tried canary, and was not able to repro the issue. But I was only able to repro the issue on the first run of M62, so could some of the test engineers also help me test this? 

test link:
http://ws9.me/ft8jtv/
https://arw.me/f/9fa0c41a59f3fd9ba62f00282f83f8.html

Cc: gov...@chromium.org
qinmin@ - looks good to me.  Your PoC is now prompting me to open the downloaded file.

govind@ - good for 63
Cc: cma...@chromium.org
+ cmasso@ (Chrome on Android TPM) for merge review and approval (PTAL comment @24). Thank you.
Labels: -Hotlist-Merge-Review -Merge-Review-63 Merge-Approved-63
Project Member

Comment 27 by bugdroid1@chromium.org, Nov 9 2017

Labels: -merge-approved-63 merge-merged-3239
The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/dd47630bbb313ca9f7efdd59ec2cf1604b2d9dbc

commit dd47630bbb313ca9f7efdd59ec2cf1604b2d9dbc
Author: Min Qin <qinmin@chromium.org>
Date: Thu Nov 09 18:53:06 2017

Only allow googlechrome protocol for http(s) urls

The protocol should only be used to navigate to http(s) urls, no content or
file uri.

BUG= 780708 
TBR=qinmin@chromium.org

(cherry picked from commit e29f7184a20c69e484adfd115b7dcdc4f774ef5b)

Change-Id: I00ea7e0fd31ec9e379ddcb5126265a6744de822d
Reviewed-on: https://chromium-review.googlesource.com/752202
Reviewed-by: Wei-Yin Chen (陳威尹) <wychen@chromium.org>
Reviewed-by: David Trainor <dtrainor@chromium.org>
Reviewed-by: Ted Choc <tedchoc@chromium.org>
Commit-Queue: Min Qin <qinmin@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#514030}
Reviewed-on: https://chromium-review.googlesource.com/761116
Reviewed-by: Min Qin <qinmin@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#434}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/dd47630bbb313ca9f7efdd59ec2cf1604b2d9dbc/chrome/android/java/src/org/chromium/chrome/browser/AppIndexingUtil.java
[modify] https://crrev.com/dd47630bbb313ca9f7efdd59ec2cf1604b2d9dbc/chrome/android/java/src/org/chromium/chrome/browser/IntentHandler.java
[modify] https://crrev.com/dd47630bbb313ca9f7efdd59ec2cf1604b2d9dbc/chrome/android/javatests/src/org/chromium/chrome/browser/IntentHandlerTest.java

Labels: -ReleaseBlock-Stable
Project Member

Comment 29 by bugdroid1@chromium.org, Nov 13 2017

The following revision refers to this bug:
  https://chromium.googlesource.com/chromium/src.git/+/76505fe06f81a71f79410ab5488949ef95a8d308

commit 76505fe06f81a71f79410ab5488949ef95a8d308
Author: Benoit Lize <lizeb@chromium.org>
Date: Mon Nov 13 09:33:46 2017

android: Add UrlUtilies.isHttpOrHttps.

This method is called by dd47630bbb313ca9f7efdd59ec2cf1604b2d9dbc which
is a cherry-pick of e29f7184a20c69e484adfd115b7dcdc4f774ef5b. This adds
the missing method to the branch.

Bug:  780708 , 783554
Change-Id: I9fd08d720f324ee825558cfac480a39f1a652d94
Reviewed-on: https://chromium-review.googlesource.com/763301
Reviewed-by: Bernhard Bauer <bauerb@chromium.org>
Cr-Commit-Position: refs/branch-heads/3239@{#458}
Cr-Branched-From: adb61db19020ed8ecee5e91b1a0ea4c924ae2988-refs/heads/master@{#508578}
[modify] https://crrev.com/76505fe06f81a71f79410ab5488949ef95a8d308/chrome/android/java/src/org/chromium/chrome/browser/util/UrlUtilities.java
[modify] https://crrev.com/76505fe06f81a71f79410ab5488949ef95a8d308/chrome/android/javatests/src/org/chromium/chrome/browser/util/UrlUtilitiesTest.java

Project Member

Comment 30 by sheriffbot@chromium.org, Feb 10 2018

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

Comment 31 by sheriffbot@chromium.org, Mar 27 2018

Labels: -Security_Impact-Beta -M-63 M-65 Security_Impact-Stable

Sign in to add a comment