Skip to content

GitLab

T
tangled
Commit 7fbf1116 authored by Rohit Agarwal's avatar Rohit Agarwal Committed by Commit Bot
Browse files
Options

Add file scheme checks for Intent URI based navigations. 

Intent URIs can expose file:// URI which upon external
navigation from can result in unhandled FileUriExposedException android
exception.

This CL adds check in ExternalNavigationHandler to ensure we never
send the intents generated by Intent URIs with file scheme to Android
which could result in unhandled exception.

This CL also adds unit and browser tests.

Bug: 1085476
Change-Id: I0cff50d517a491dc27e345372ddcc5c77f8e26cb
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2212169
Commit-Queue: Rohit Agarwal <roagarwal@chromium.org>
Reviewed-by: default avatarMichael Thiessen <mthiesse@chromium.org>
Cr-Commit-Position: refs/heads/master@{#771319}
parent 55015efd
Show whitespace changes
Inline Side-by-side
Showing with 128 additions and 7 deletions
chrome/android/javatests/src/org/chromium/chrome/browser/externalnav/UrlOverridingTest.java
View file @ 7fbf1116
...@@ -97,6 +97,8 @@ public class UrlOverridingTest { ...@@ -97,6 +97,8 @@ public class UrlOverridingTest {
BASE_PATH + "navigation_from_java_redirection.html"; BASE_PATH + "navigation_from_java_redirection.html";
private static final String NAVIGATION_TO_CCT_FROM_INTENT_URI = private static final String NAVIGATION_TO_CCT_FROM_INTENT_URI =
BASE_PATH + "navigation_to_cct_via_intent_uri.html"; BASE_PATH + "navigation_to_cct_via_intent_uri.html";
private static final String NAVIGATION_TO_FILE_SCHEME_FROM_INTENT_URI =
BASE_PATH + "navigation_to_file_scheme_via_intent_uri.html";
private static class TestTabObserver extends EmptyTabObserver { private static class TestTabObserver extends EmptyTabObserver {
private final CallbackHelper mFinishCallback; private final CallbackHelper mFinishCallback;
...@@ -490,4 +492,37 @@ public class UrlOverridingTest { ...@@ -490,4 +492,37 @@ public class UrlOverridingTest {
loadUrlAndWaitForIntentUrl(originalUrl, true, false, false, fallbackUrl, true); loadUrlAndWaitForIntentUrl(originalUrl, true, false, false, fallbackUrl, true);
} }
@Test
@LargeTest
public void testIntentURIWithFileSchemeDoesNothing() throws TimeoutException {
mActivityTestRule.startMainActivityOnBlankPage();
String originalUrl = mTestServer.getURL(NAVIGATION_TO_FILE_SCHEME_FROM_INTENT_URI);
loadUrlAndWaitForIntentUrl(originalUrl, true, false, false, null, false, "scheme_file");
}
@Test
@LargeTest
public void testIntentURIWithMixedCaseFileSchemeDoesNothing() throws TimeoutException {
mActivityTestRule.startMainActivityOnBlankPage();
String originalUrl = mTestServer.getURL(NAVIGATION_TO_FILE_SCHEME_FROM_INTENT_URI);
loadUrlAndWaitForIntentUrl(
originalUrl, true, false, false, null, false, "scheme_mixed_case_file");
}
@Test
@LargeTest
public void testIntentURIWithNoSchemeDoesNothing() throws TimeoutException {
mActivityTestRule.startMainActivityOnBlankPage();
String originalUrl = mTestServer.getURL(NAVIGATION_TO_FILE_SCHEME_FROM_INTENT_URI);
loadUrlAndWaitForIntentUrl(originalUrl, true, false, false, null, false, "null_scheme");
}
@Test
@LargeTest
public void testIntentURIWithEmptySchemeDoesNothing() throws TimeoutException {
mActivityTestRule.startMainActivityOnBlankPage();
String originalUrl = mTestServer.getURL(NAVIGATION_TO_FILE_SCHEME_FROM_INTENT_URI);
loadUrlAndWaitForIntentUrl(originalUrl, true, false, false, null, false, "empty_scheme");
}
} }
\ No newline at end of file
chrome/test/data/android/url_overriding/navigation_to_file_scheme_via_intent_uri.html 0 → 100644
View file @ 7fbf1116
<!DOCTYPE html>
<html>
<!--
Intent URI example taken from crbug.com/1056754 with package modification here.
-->
<head>
<meta name="viewport"
content="width=device-width, initial-scale=1.0, maximum-scale=1.0" />
</head>
<body>
<a id="scheme_file" target='_blank' href='intent:///x.mhtml#Intent;package=org.chromium.chrome.tests;action=android.intent.action.VIEW;scheme=file;end;'>
Intent URI with scheme file!!
</a> <br>
<a id="scheme_mixed_case_file" target='_blank' href='intent:///x.mhtml#Intent;package=org.chromium.chrome.tests;action=android.intent.action.VIEW;scheme=FiLe;end;'>
Intent URI with scheme FiLe!!
</a><br>
<a id="null_scheme" target='_blank' href='intent:///x.mhtml#Intent;package=org.chromium.chrome.tests;action=android.intent.action.VIEW;end;'>
Intent URI with NULL scheme!!
</a><br>
<a id="empty_scheme" target='_blank' href='intent:///x.mhtml#Intent;package=org.chromium.chrome.tests;action=android.intent.action.VIEW;scheme=;end;'>
Intent URI with empty scheme!!
</a><br>
</body>
</html>
\ No newline at end of file
components/external_intents/android/java/src/org/chromium/components/external_intents/ExternalNavigationHandler.java
View file @ 7fbf1116
...@@ -650,6 +650,30 @@ public class ExternalNavigationHandler { ...@@ -650,6 +650,30 @@ public class ExternalNavigationHandler {
return true; return true;
} }
/**
* Intent URIs leads to creating intents that chrome would use for firing external navigations
* via Android. Android throws an exception [1] when an application exposes a file:// Uri to
* another app.
*
* This method checks if the |targetIntent| contains the file:// scheme in its data.
*
* [1]: https://developer.android.com/reference/android/os/FileUriExposedException
*/
private boolean hasFileSchemeInIntentURI(Intent targetIntent, boolean hasIntentScheme) {
// We are only concerned with targetIntent that was generated due to intent:// schemes only.
if (!hasIntentScheme) return false;
Uri data = targetIntent.getData();
if (data == null || data.getScheme() == null) return false;
if (data.getScheme().equalsIgnoreCase(UrlConstants.FILE_SCHEME)) {
if (DEBUG) Log.i(TAG, "Intent navigation to file: URI");
return true;
}
return false;
}
/** /**
* Special case - It makes no sense to use an external application for a YouTube * Special case - It makes no sense to use an external application for a YouTube
* pairing code URL, since these match the current tab with a device (Chromecast * pairing code URL, since these match the current tab with a device (Chromecast
...@@ -1016,6 +1040,10 @@ public class ExternalNavigationHandler { ...@@ -1016,6 +1040,10 @@ public class ExternalNavigationHandler {
return OverrideUrlLoadingResult.NO_OVERRIDE; return OverrideUrlLoadingResult.NO_OVERRIDE;
} }
if (hasFileSchemeInIntentURI(targetIntent, hasIntentScheme)) {
return OverrideUrlLoadingResult.NO_OVERRIDE;
}
if (isYoutubePairingCode(params)) return OverrideUrlLoadingResult.NO_OVERRIDE; if (isYoutubePairingCode(params)) return OverrideUrlLoadingResult.NO_OVERRIDE;
if (shouldStayInIncognito(params, isExternalProtocol)) { if (shouldStayInIncognito(params, isExternalProtocol)) {
......
components/external_intents/android/javatests/src/org/chromium/components/external_intents/ExternalNavigationHandlerTest.java
View file @ 7fbf1116
...@@ -331,16 +331,13 @@ public class ExternalNavigationHandlerTest { ...@@ -331,16 +331,13 @@ public class ExternalNavigationHandlerTest {
@SmallTest @SmallTest
public void testIgnore() { public void testIgnore() {
// Ensure the following URLs are not broadcast for external navigation. // Ensure the following URLs are not broadcast for external navigation.
String urlsToIgnore[] = new String[] { String urlsToIgnore[] = new String[] {"about:test",
"about:test",
"content:test", // Content URLs should not be exposed outside of Chrome. "content:test", // Content URLs should not be exposed outside of Chrome.
"chrome://history", "chrome://history", "chrome-native://newtab", "devtools://foo",
"chrome-native://newtab",
"devtools://foo",
"intent:chrome-urls#Intent;package=com.android.chrome;scheme=about;end;", "intent:chrome-urls#Intent;package=com.android.chrome;scheme=about;end;",
"intent:chrome-urls#Intent;package=com.android.chrome;scheme=chrome;end;", "intent:chrome-urls#Intent;package=com.android.chrome;scheme=chrome;end;",
"intent://com.android.chrome.FileProvider/foo.html#Intent;scheme=content;end;", "intent://com.android.chrome.FileProvider/foo.html#Intent;scheme=content;end;",
}; "intent:///x.mhtml#Intent;package=com.android.chrome;action=android.intent.action.VIEW;scheme=file;end;"};
for (String url : urlsToIgnore) { for (String url : urlsToIgnore) {
checkUrl(url).expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE); checkUrl(url).expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE);
checkUrl(url).withIsIncognito(true).expecting( checkUrl(url).withIsIncognito(true).expecting(
...@@ -1253,6 +1250,37 @@ public class ExternalNavigationHandlerTest { ...@@ -1253,6 +1250,37 @@ public class ExternalNavigationHandlerTest {
mDelegate.startActivityIntent.getFlags()); mDelegate.startActivityIntent.getFlags());
} }
@Test
@SmallTest
public void testIntentWithFileSchemeFiltered() {
checkUrl("intent://#Intent;package=com.test.package;scheme=file;end;")
.expecting(OverrideUrlLoadingResult.NO_OVERRIDE, IGNORE);
}
@Test
@SmallTest
public void testIntentWithNoSchemeLaunched() {
checkUrl("intent://#Intent;package=com.test.package;end;")
.expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
START_OTHER_ACTIVITY);
}
@Test
@SmallTest
public void testIntentWithEmptySchemeLaunched() {
checkUrl("intent://#Intent;package=com.test.package;scheme=;end;")
.expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
START_OTHER_ACTIVITY);
}
@Test
@SmallTest
public void testIntentWithWeirdSchemeLaunched() {
checkUrl("intent://#Intent;package=com.test.package;scheme=w3irD;end;")
.expecting(OverrideUrlLoadingResult.OVERRIDE_WITH_EXTERNAL_INTENT,
START_OTHER_ACTIVITY);
}
@Test @Test
@SmallTest @SmallTest
public void testIntentWithMissingReferrer() { public void testIntentWithMissingReferrer() {
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment