Issue metadata
Sign in to add a comment
|
Pwn2own: RenderViewImpl::LaunchAndroidContentIntent in renderer can open arbitrary content intent scheme urls |
||||||||||||||||||||||||||||||
Issue description
By inspecting the source of src/content/renderer/render_view_impl.cc , there is a function called
RendererViewImpl::LaunchAndroidContentClient , which is called when phone
numbers/addresses are recognized and specific tel::// intent is send out to invoke SMS/Phone activities.
The OnStartContentIntent function then calls back and forth into Java code
sandbox escape in chrome android
void RenderViewImpl::LaunchAndroidContentIntent(const GURL& intent,
size_t request_id,
bool is_main_frame) {
if (request_id != expected_content_intent_id_)
return;
// Remove the content highlighting if any.
ScheduleComposite();
if (!intent.is_empty()) {
base::RecordAction(base::UserMetricsAction(
"Android.ContentDetectorActivated"));
Send(new ViewHostMsg_StartContentIntent(GetRoutingID(), intent,
is_main_frame));
}
}
bool RenderWidgetHostViewAndroid::OnMessageReceived(
const IPC::Message& message) {
if (IPC_MESSAGE_ID_CLASS(message.type()) == SyncCompositorMsgStart) {
return SyncCompositorOnMessageReceived(message);
}
bool handled = true;
IPC_BEGIN_MESSAGE_MAP(RenderWidgetHostViewAndroid, message)
IPC_MESSAGE_HANDLER(ViewHostMsg_StartContentIntent, OnStartContentIntent)
IPC_MESSAGE_HANDLER(ViewHostMsg_SmartClipDataExtracted,
OnSmartClipDataExtracted)
IPC_MESSAGE_HANDLER(ViewHostMsg_ShowUnhandledTapUIIfNeeded,
OnShowUnhandledTapUIIfNeeded)
IPC_MESSAGE_UNHANDLED(handled = false)
IPC_END_MESSAGE_MAP()
return handled;
}
src/content/browser/renderer_host/render_widget_host_view_android.cc :
void RenderWidgetHostViewAndroid::OnStartContentIntent(
const GURL& content_url, bool is_main_frame) {
if (content_view_core_)
content_view_core_->StartContentIntent(content_url, is_main_frame);
}
//...
src/content/browser/android/content_view_core_impl.cc
void ContentViewCoreImpl::StartContentIntent(const GURL& content_url,
bool is_main_frame) {
JNIEnv* env = AttachCurrentThread();
ScopedJavaLocalRef<jobject> j_obj = java_ref_.get(env);
if (j_obj.is_null())
return;
ScopedJavaLocalRef<jstring> jcontent_url =
ConvertUTF8ToJavaString(env, content_url.spec());
Java_ContentViewCore_startContentIntent(env, j_obj, jcontent_url,
is_main_frame);
}
//...
//content/public/android/java/src/org/chromium/content/browser/ContentViewCore.java
@CalledByNative
private void startContentIntent(String contentUrl, boolean isMainFrame) {
getContentViewClient().onStartContentIntent(getContext(), contentUrl, isMainFrame);
}
//src/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
public void onStartContentIntent(Context context, String intentUrl, boolean isMainFrame) {
Intent intent;
// Perform generic parsing of the URI to turn it into an Intent.
try {
intent = Intent.parseUri(intentUrl, Intent.URI_INTENT_SCHEME);
intent.addFlags(Intent.FLAG_ACTIVITY_NEW_TASK);
} catch (Exception ex) {
Log.w(TAG, "Bad URI %s", intentUrl, ex);
return;
}
try {
context.startActivity(intent);
} catch (ActivityNotFoundException ex) {
Log.w(TAG, "No application can handle %s", intentUrl);
}
}
The incoming intent scheme url is pared by parseUri to an Intent, and the target activity is started by
broker process. However as this message comes from render, compromised render can control the intent
scheme url, which in normal situations should be an implicit intent such as tel:// or sms://, to an explicit
intent with component specified and string extras payload.
In short, a compromised renderer process can start arbitrary activity in the system, note payloads can only
be represented as strings, due to the limitation of parseUri. That means you cannot pass a Parcelable, but
can pass a StringExtra.
,
Oct 26 2016
Ted, this is Pri-0 exploit bug from Pwn2own mobile conference. Can you please help to triage and find owner for this. Looks like we are missing intent validation on browser process side here.
,
Oct 26 2016
,
Oct 26 2016
I'm OOO for a few more days. @Maria, can you take a peek? + qinmin@ since this has to do with content detectors + twellington@ since we just talked about deleting these. I think we can look at the intents the detectors generator and add validation that it matches those. Still would allow a renderer to initiate a phone call or something that is allowed from the renderer. @Maria, do we require user gesture to launch an external intent via navigations? It doesn't look like we add browsable as a category to the intent, but otherwise doesn't seem any different than our normal external intent dispatch (but maybe user gesture and the category is the difference).
,
Oct 26 2016
,
Oct 26 2016
,
Oct 26 2016
,
Oct 26 2016
Minimum fix for this to atleast whitelist the schemes allowed for intent on browser side, can we have that in soon please. Added bonus will be to add the user gesture check.
,
Oct 26 2016
From the quick read, this is not related to the content detection code. The attack happens when renderer is first comprised, then it sends IPC message, ViewHostMsg_StartContentIntent, with its own url. As user gesture bit is also coming from the renderer, so I assume we can't trust it neither. Whitelist the allowed scheme for the intentUrl seems a quick fix for now. Unfortunately we can not add browsable category as these are not http(s) and the intent handler won't put browsable as category.
,
Oct 26 2016
FWIW, we've decided to delete content intents as a whole soon: https://groups.google.com/a/google.com/forum/#!topic/contextual-search-eng/1Uo7aej3FwE . So I'd suggest going with a minimal fix now (like scheme whitelist) to tide us over and treat the deletion as the long-term fix.
,
Oct 26 2016
Sgtm, can someone add the whitelist please.
,
Oct 26 2016
Maria is out sick today. I can add the whitelist.
,
Oct 26 2016
Thanks Theresa, really appreciate your help here.
,
Oct 26 2016
,
Oct 26 2016
CL is out for review: https://codereview.chromium.org/2448363003/
,
Oct 26 2016
,
Oct 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/041cce95c89455f4243360d3303113daf1c5df30 commit 041cce95c89455f4243360d3303113daf1c5df30 Author: twellington <twellington@chromium.org> Date: Wed Oct 26 18:44:40 2016 Add scheme whitelist for content intents Add a whitelist for content intents sent when the user taps on an address, email address, or phone number. BUG= 659477 Review-Url: https://codereview.chromium.org/2448363003 Cr-Commit-Position: refs/heads/master@{#427758} [modify] https://crrev.com/041cce95c89455f4243360d3303113daf1c5df30/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
,
Oct 26 2016
,
Oct 26 2016
[Automated comment] Request affecting a post-stable build (M54), manual review required.
,
Oct 26 2016
Your change meets the bar and is auto-approved for M55 (branch: 2883)
,
Oct 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/abd993bfcdc18d41e5ea0f34312543bd6dae081e commit abd993bfcdc18d41e5ea0f34312543bd6dae081e Author: Theresa Wellington <twellington@google.com> Date: Wed Oct 26 18:59:02 2016 Add scheme whitelist for content intents Add a whitelist for content intents sent when the user taps on an address, email address, or phone number. BUG= 659477 TBR=aelias@chromium.org Review URL: https://codereview.chromium.org/2455753002 . Review-Url: https://codereview.chromium.org/2448363003 Cr-Original-Commit-Position: refs/heads/master@{#427758} Cr-Commit-Position: refs/branch-heads/2840@{#778} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/abd993bfcdc18d41e5ea0f34312543bd6dae081e/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
,
Oct 26 2016
,
Oct 26 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9956ee683d2d2832c943602e8efa56f923f23492 commit 9956ee683d2d2832c943602e8efa56f923f23492 Author: Theresa Wellington <twellington@google.com> Date: Wed Oct 26 21:28:19 2016 Add scheme whitelist for content intents Add a whitelist for content intents sent when the user taps on an address, email address, or phone number. BUG= 659477 TBR=aelias@chromium.org Review URL: https://codereview.chromium.org/2448113006 . Review-Url: https://codereview.chromium.org/2448363003 Cr-Original-Commit-Position: refs/heads/master@{#427758} Cr-Commit-Position: refs/branch-heads/2883@{#316} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/9956ee683d2d2832c943602e8efa56f923f23492/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
,
Oct 26 2016
,
Oct 27 2016
,
Oct 27 2016
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/9956ee683d2d2832c943602e8efa56f923f23492 commit 9956ee683d2d2832c943602e8efa56f923f23492 Author: Theresa Wellington <twellington@google.com> Date: Wed Oct 26 21:28:19 2016 Add scheme whitelist for content intents Add a whitelist for content intents sent when the user taps on an address, email address, or phone number. BUG= 659477 TBR=aelias@chromium.org Review URL: https://codereview.chromium.org/2448113006 . Review-Url: https://codereview.chromium.org/2448363003 Cr-Original-Commit-Position: refs/heads/master@{#427758} Cr-Commit-Position: refs/branch-heads/2883@{#316} Cr-Branched-From: 614d31daee2f61b0180df403a8ad43f20b9f6dd7-refs/heads/master@{#423768} [modify] https://crrev.com/9956ee683d2d2832c943602e8efa56f923f23492/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
,
Oct 27 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/abd993bfcdc18d41e5ea0f34312543bd6dae081e commit abd993bfcdc18d41e5ea0f34312543bd6dae081e Author: Theresa Wellington <twellington@google.com> Date: Wed Oct 26 18:59:02 2016 Add scheme whitelist for content intents Add a whitelist for content intents sent when the user taps on an address, email address, or phone number. BUG= 659477 TBR=aelias@chromium.org Review URL: https://codereview.chromium.org/2455753002 . Review-Url: https://codereview.chromium.org/2448363003 Cr-Original-Commit-Position: refs/heads/master@{#427758} Cr-Commit-Position: refs/branch-heads/2840@{#778} Cr-Branched-From: 1ae106dbab4bddd85132d5b75c670794311f4c57-refs/heads/master@{#414607} [modify] https://crrev.com/abd993bfcdc18d41e5ea0f34312543bd6dae081e/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
,
Oct 28 2016
,
Oct 31 2016
,
Oct 31 2016
,
Nov 8 2016
,
Nov 9 2016
+WebView folks Is it possible that this exploit affects WebView as well? If so, should we add the scheme whitelists added to ContentViewClient in a WebView equivalent place?
,
Nov 9 2016
,
Nov 11 2016
,
Nov 11 2016
,
Nov 14 2016
I'm not sure what you mean by "added in a WebView equivalent place" - it looks like all this code is in content and not chrome-specific. We should probably verify that this fix also works for WebView, though it's only relevant in the multiprocess webview, since in the default single-process webview, there's already no security boundary between the renderer and browser and so an RCE is already fatal :(
,
Nov 14 2016
AwContentViewClient.java overrides the method in ContentViewClient.java that the scheme whitelist was added to, so while the code is in content, the handling of the content intent is different in webview vs regular chrome. I'm not familiar with how webview handles content intents (beyond knowing where in the code lives) or how to repro the compromised renderer to test this in webview. Adding the whitelist is really easy, though, so we should go ahead and do that if it's something that we think may needed.
,
Nov 15 2016
,
Nov 15 2016
Ah, thanks. WebView's implementation should be fixed in the same way, However, this isn't an immediate security issue, because in single-process webview a renderer RCE is already game over. Multiprocess webview is only available as an off-by-default developer option in N. One related question, though: what stops the compromised renderer from achieving the same goal without needing LaunchAndroidContentIntent at all by simply pretending that the user clicked on a link whose target URI is one that the app in question has a browsable intent filter for? e.g. just <a href="asdfasdf:foo"> or similar?
,
Nov 23 2016
have the same question as Torne here. Does anybody have an answer?
,
Nov 24 2016
The following revision refers to this bug: https://chromium.googlesource.com/chromium/src.git/+/1825e488888decae9be34ea17a22c92a5e890874 commit 1825e488888decae9be34ea17a22c92a5e890874 Author: torne <torne@chromium.org> Date: Thu Nov 24 15:19:56 2016 WebView: Add scheme whitelist for content intents. Use the content intent scheme whitelist introduced in https://codereview.chromium.org/2448363003 in WebView, which overrides the default implementation. BUG= 659477 Review-Url: https://codereview.chromium.org/2524843003 Cr-Commit-Position: refs/heads/master@{#434325} [modify] https://crrev.com/1825e488888decae9be34ea17a22c92a5e890874/android_webview/java/src/org/chromium/android_webview/AwContentViewClient.java [modify] https://crrev.com/1825e488888decae9be34ea17a22c92a5e890874/content/public/android/java/src/org/chromium/content/browser/ContentViewClient.java
,
Nov 24 2016
Landed the equivalent change for WebView in master; I won't cherrypick it to release branches as it's only security-relevant for WebView in multiprocess, which is still only a developer option. I do still want to know the answer to my question in #40 though: is there anything preventing the exploit chain from simply faking a click on a link that leads to a scheme that the app has registered an intent filter for to get it to launch? And if so, what is it, because we may want to think about how that affects WebView..
,
Nov 28 2016
Aha, okay. I re-read the original exploit writeup and I was misunderstanding the sequence of events in the exploit. The problem was that certain apps with privileged access to the Google signin cookie exposed activities that would open arbitrary URLs in WebView. These activities are *not* marked BROWSABLE and so cannot be launched via the regular "link click" IPCs. So, yes, fixing the content intent IPC here was critical to prevent that step of the exploit.
,
Feb 2 2017
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
,
Apr 25 2018
,
Jul 28
|
|||||||||||||||||||||||||||||||
►
Sign in to add a comment |
|||||||||||||||||||||||||||||||
Comment 1 by infe...@chromium.org
, Oct 26 2016