From ac1f9c790adf9fab051ce1f5b3bcea1deb3e6fee Mon Sep 17 00:00:00 2001 From: Ian Clelland Date: Wed, 22 Oct 2014 16:16:52 -0400 Subject: [PATCH] Defer whitelist decisions to plugins There is a default policy, which is implemented in the case where no plugins override any of the whitelist methods: * Error URLs must start with file:// * Navigation is allowed to file:// and data: URLs which do not contain "app_webview" * External URLs do not launch intents * XHRs are allowed to file:// and data: URLs which do not contain "app_webview" --- .../org/apache/cordova/AndroidWebView.java | 8 +- .../org/apache/cordova/CordovaActivity.java | 8 +- .../src/org/apache/cordova/CordovaBridge.java | 9 +- .../org/apache/cordova/CordovaUriHelper.java | 94 +++++++++++++++---- .../cordova/IceCreamCordovaWebViewClient.java | 7 +- 5 files changed, 96 insertions(+), 30 deletions(-) diff --git a/framework/src/org/apache/cordova/AndroidWebView.java b/framework/src/org/apache/cordova/AndroidWebView.java index 6ad1af6d..d9f5e149 100755 --- a/framework/src/org/apache/cordova/AndroidWebView.java +++ b/framework/src/org/apache/cordova/AndroidWebView.java @@ -92,6 +92,7 @@ public class AndroidWebView extends WebView implements CordovaWebView { private Whitelist externalWhitelist; private CordovaPreferences preferences; private CoreAndroid appPlugin; + private CordovaUriHelper helper; // The URL passed to loadUrl(), not necessarily the URL of the current page. String loadedUrl; @@ -123,6 +124,7 @@ public class AndroidWebView extends WebView implements CordovaWebView { this.internalWhitelist = internalWhitelist; this.externalWhitelist = externalWhitelist; this.preferences = preferences; + this.helper = new CordovaUriHelper(cordova, this); pluginManager = new PluginManager(this, this.cordova, pluginEntries); cookieManager = new AndroidCookieManager(this); @@ -141,7 +143,7 @@ public class AndroidWebView extends WebView implements CordovaWebView { cordova.getActivity().runOnUiThread(r); } })); - bridge = new CordovaBridge(pluginManager, nativeToJsMessageQueue, this.cordova.getActivity().getPackageName()); + bridge = new CordovaBridge(pluginManager, nativeToJsMessageQueue, this.cordova.getActivity().getPackageName(), helper); initWebViewSettings(); pluginManager.addService(CoreAndroid.PLUGIN_NAME, CoreAndroid.class.getCanonicalName()); pluginManager.init(); @@ -386,7 +388,7 @@ public class AndroidWebView extends WebView implements CordovaWebView { if (LOG.isLoggable(LOG.DEBUG) && !url.startsWith("javascript:")) { LOG.d(TAG, ">>> loadUrlNow()"); } - if (url.startsWith("file://") || url.startsWith("javascript:") || url.startsWith("about:") || internalWhitelist.isUrlWhiteListed(url)) { + if (url.startsWith("javascript:") || helper.shouldAllowNavigation(url)) { super.loadUrl(url); } } @@ -461,7 +463,7 @@ public class AndroidWebView extends WebView implements CordovaWebView { if (!openExternal) { // Make sure url is in whitelist - if (url.startsWith("file://") || internalWhitelist.isUrlWhiteListed(url)) { + if (helper.shouldAllowNavigation(url)) { // TODO: What about params? // Load new URL loadUrlIntoView(url, true); diff --git a/framework/src/org/apache/cordova/CordovaActivity.java b/framework/src/org/apache/cordova/CordovaActivity.java index 6874f5b2..0562cdd1 100755 --- a/framework/src/org/apache/cordova/CordovaActivity.java +++ b/framework/src/org/apache/cordova/CordovaActivity.java @@ -142,7 +142,7 @@ public class CordovaActivity extends Activity { appView = makeWebView(); createViews(); //TODO: Add null check against CordovaInterfaceImpl, since this can be fragile - appView.init(cordovaInterface, pluginEntries, internalWhitelist, externalWhitelist, preferences); + appView.init(cordovaInterface, pluginEntries, preferences); cordovaInterface.setPluginManager(appView.getPluginManager()); // Wire the hardware volume controls to control media if desired. @@ -354,7 +354,11 @@ public class CordovaActivity extends Activity { // If errorUrl specified, then load it final String errorUrl = preferences.getString("errorUrl", null); - if ((errorUrl != null) && (errorUrl.startsWith("file://") || internalWhitelist.isUrlWhiteListed(errorUrl)) && (!failingUrl.equals(errorUrl))) { + CordovaUriHelper helper = new CordovaUriHelper(this.cordovaInterface, appView); + if ((errorUrl != null) && + (!failingUrl.equals(errorUrl)) && + (appView != null && helper.shouldAllowNavigation(errorUrl)) + ) { // Load URL on UI thread me.runOnUiThread(new Runnable() { public void run() { diff --git a/framework/src/org/apache/cordova/CordovaBridge.java b/framework/src/org/apache/cordova/CordovaBridge.java index becbd529..f581cf01 100644 --- a/framework/src/org/apache/cordova/CordovaBridge.java +++ b/framework/src/org/apache/cordova/CordovaBridge.java @@ -38,11 +38,13 @@ public class CordovaBridge { private volatile int expectedBridgeSecret = -1; // written by UI thread, read by JS thread. private String loadedUrl; private String appContentUrlPrefix; + protected CordovaUriHelper helper; - public CordovaBridge(PluginManager pluginManager, NativeToJsMessageQueue jsMessageQueue, String packageName) { + public CordovaBridge(PluginManager pluginManager, NativeToJsMessageQueue jsMessageQueue, String packageName, CordovaUriHelper helper) { this.pluginManager = pluginManager; this.jsMessageQueue = jsMessageQueue; this.appContentUrlPrefix = "content://" + packageName + "."; + this.helper = helper; } public String jsExec(int bridgeSecret, String service, String action, String callbackId, String arguments) throws JSONException, IllegalAccessException { @@ -167,11 +169,14 @@ public class CordovaBridge { } else if (defaultValue != null && defaultValue.startsWith("gap_init:")) { // Protect against random iframes being able to talk through the bridge. + // Trust only file URLs and pages which the app would have been allowed + // to navigate to anyway. // Trust only file URLs and the start URL's domain. // The extra origin.startsWith("http") is to protect against iframes with data: having "" as origin. if (origin.startsWith("file:") || origin.startsWith(this.appContentUrlPrefix) || - (origin.startsWith("http") && loadedUrl.startsWith(origin))) { + (origin.startsWith("http") && loadedUrl.startsWith(origin)) || + helper.shouldAllowNavigation(origin)) { // Enable the bridge int bridgeMode = Integer.parseInt(defaultValue.substring(9)); jsMessageQueue.setBridgeMode(bridgeMode); diff --git a/framework/src/org/apache/cordova/CordovaUriHelper.java b/framework/src/org/apache/cordova/CordovaUriHelper.java index 6c1c4fa9..9aea8176 100644 --- a/framework/src/org/apache/cordova/CordovaUriHelper.java +++ b/framework/src/org/apache/cordova/CordovaUriHelper.java @@ -23,6 +23,7 @@ import android.annotation.TargetApi; import android.content.Intent; import android.net.Uri; import android.os.Build; +import android.util.Log; import android.webkit.WebView; public class CordovaUriHelper { @@ -37,11 +38,77 @@ public class CordovaUriHelper { appView = webView; cordova = cdv; } - + + /** + * Determine whether the webview should be allowed to navigate to a given URL. + * + * This method implements the default whitelist policy when no plugins override + * shouldAllowNavigation + */ + public boolean shouldAllowNavigation(String url) { + Boolean pluginManagerAllowsNavigation = this.appView.getPluginManager().shouldAllowNavigation(url); + if (pluginManagerAllowsNavigation == null) { + // Default policy: + // Internal urls on file:// or data:// that do not contain "app_webview" are allowed for navigation + if(url.startsWith("file://") || url.startsWith("data:")) + { + //This directory on WebKit/Blink based webviews contains SQLite databases! + //DON'T CHANGE THIS UNLESS YOU KNOW WHAT YOU'RE DOING! + return !url.contains("app_webview"); + } + return false; + } + return pluginManagerAllowsNavigation; + } + + /** + * Determine whether the webview should be allowed to launch an intent for a given URL. + * + * This method implements the default whitelist policy when no plugins override + * shouldOpenExternalUrl + */ + public boolean shouldOpenExternalUrl(String url) { + Boolean pluginManagerAllowsExternalUrl = this.appView.getPluginManager().shouldOpenExternalUrl(url); + if (pluginManagerAllowsExternalUrl == null) { + // Default policy: + // External URLs are not allowed + return false; + } + return pluginManagerAllowsExternalUrl; + } + + /** + * Determine whether the webview should be allowed to request a resource from a given URL. + * + * This method implements the default whitelist policy when no plugins override + * shouldAllowRequest + */ + public boolean shouldAllowRequest(String url) { + + Boolean pluginManagerAllowsRequest = this.appView.getPluginManager().shouldAllowRequest(url); + if (pluginManagerAllowsRequest == null) { + // Default policy: + // Internal urls on file:// or data:// that do not contain "app_webview" are allowed for navigation + if(url.startsWith("file://") || url.startsWith("data:")) + { + //This directory on WebKit/Blink based webviews contains SQLite databases! + //DON'T CHANGE THIS UNLESS YOU KNOW WHAT YOU'RE DOING! + return !url.contains("app_webview"); + } + return false; + } + return pluginManagerAllowsRequest; + } + /** * Give the host application a chance to take over the control when a new url * is about to be loaded in the current WebView. * + * This method implements the default whitelist policy when no plugins override + * the whitelist methods: + * Internal urls on file:// or data:// that do not contain "app_webview" are allowed for navigation + * External urls are not allowed. + * * @param view The WebView that is initiating the callback. * @param url The url to be loaded. * @return true to override, false for default behavior @@ -49,23 +116,15 @@ public class CordovaUriHelper { @TargetApi(Build.VERSION_CODES.ICE_CREAM_SANDWICH_MR1) public boolean shouldOverrideUrlLoading(String url) { // Give plugins the chance to handle the url - if (this.appView.getPluginManager().onOverrideUrlLoading(url)) { - // Do nothing other than what the plugins wanted. - // If any returned true, then the request was handled. - return true; - } - else if(url.startsWith("file://") | url.startsWith("data:")) - { - //This directory on WebKit/Blink based webviews contains SQLite databases! - //DON'T CHANGE THIS UNLESS YOU KNOW WHAT YOU'RE DOING! - return url.contains("app_webview"); - } - else if (appView.getWhitelist().isUrlWhiteListed(url)) { + if (shouldAllowNavigation(url)) { // Allow internal navigation return false; } - else if (appView.getExternalWhitelist().isUrlWhiteListed(url)) - { + if (shouldOpenExternalUrl(url)) { + // Do nothing other than what the plugins wanted. + // If any returned false, then the request was either blocked + // completely, or handled out-of-band by the plugin. If they all + // returned true, then we should open the URL here. try { Intent intent = new Intent(Intent.ACTION_VIEW); intent.setData(Uri.parse(url)); @@ -77,10 +136,11 @@ public class CordovaUriHelper { this.cordova.getActivity().startActivity(intent); return true; } catch (android.content.ActivityNotFoundException e) { - LOG.e(TAG, "Error loading url " + url, e); + Log.e(TAG, "Error loading url " + url, e); } + return true; } - // Intercept the request and do nothing with it -- block it + // Block by default return true; } } diff --git a/framework/src/org/apache/cordova/IceCreamCordovaWebViewClient.java b/framework/src/org/apache/cordova/IceCreamCordovaWebViewClient.java index 68f87419..33b7bace 100644 --- a/framework/src/org/apache/cordova/IceCreamCordovaWebViewClient.java +++ b/framework/src/org/apache/cordova/IceCreamCordovaWebViewClient.java @@ -45,7 +45,7 @@ public class IceCreamCordovaWebViewClient extends AndroidWebViewClient { try { // Check the against the whitelist and lock out access to the WebView directory // Changing this will cause problems for your application - if (isUrlHarmful(url)) { + if (!helper.shouldAllowRequest(url)) { LOG.w(TAG, "URL blocked by whitelist: " + url); // Results in a 404. return new WebResourceResponse("text/plain", "UTF-8", null); @@ -71,11 +71,6 @@ public class IceCreamCordovaWebViewClient extends AndroidWebViewClient { } } - private boolean isUrlHarmful(String url) { - return ((url.startsWith("http:") || url.startsWith("https:")) && !appView.getWhitelist().isUrlWhiteListed(url)) - || url.contains("app_webview"); - } - private static boolean needsKitKatContentUrlFix(Uri uri) { return android.os.Build.VERSION.SDK_INT >= android.os.Build.VERSION_CODES.KITKAT && "content".equals(uri.getScheme()); }