cordova-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From i..@apache.org
Subject [3/7] android commit: Defer whitelist decisions to plugins
Date Thu, 30 Oct 2014 16:20:38 GMT
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"


Project: http://git-wip-us.apache.org/repos/asf/cordova-android/repo
Commit: http://git-wip-us.apache.org/repos/asf/cordova-android/commit/23584274
Tree: http://git-wip-us.apache.org/repos/asf/cordova-android/tree/23584274
Diff: http://git-wip-us.apache.org/repos/asf/cordova-android/diff/23584274

Branch: refs/heads/unplug-whitelist
Commit: 23584274d29035dfcc88df36f80d284d4ac740f1
Parents: 44aa988
Author: Ian Clelland <iclelland@chromium.org>
Authored: Wed Oct 22 16:16:52 2014 -0400
Committer: Ian Clelland <iclelland@chromium.org>
Committed: Thu Oct 30 12:19:06 2014 -0400

----------------------------------------------------------------------
 .../src/org/apache/cordova/AndroidWebView.java  |  8 +-
 .../src/org/apache/cordova/CordovaActivity.java |  6 +-
 .../src/org/apache/cordova/CordovaBridge.java   | 12 +--
 .../org/apache/cordova/CordovaUriHelper.java    | 94 ++++++++++++++++----
 .../cordova/IceCreamCordovaWebViewClient.java   |  7 +-
 5 files changed, 95 insertions(+), 32 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/cordova-android/blob/23584274/framework/src/org/apache/cordova/AndroidWebView.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/AndroidWebView.java b/framework/src/org/apache/cordova/AndroidWebView.java
index 9817476..36ef39f 100755
--- a/framework/src/org/apache/cordova/AndroidWebView.java
+++ b/framework/src/org/apache/cordova/AndroidWebView.java
@@ -89,6 +89,7 @@ public class AndroidWebView extends WebView implements CordovaWebView {
     private Whitelist internalWhitelist;
     private Whitelist externalWhitelist;
     private CordovaPreferences preferences;
+    private CordovaUriHelper helper;
     // The URL passed to loadUrl(), not necessarily the URL of the current page.
     String loadedUrl;
     
@@ -120,10 +121,11 @@ 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);
         resourceApi = new CordovaResourceApi(this.getContext(), pluginManager);
-        bridge = new CordovaBridge(pluginManager, new NativeToJsMessageQueue(this, cordova));
+        bridge = new CordovaBridge(pluginManager, new NativeToJsMessageQueue(this, cordova),
helper);
         pluginManager.addService("App", "org.apache.cordova.CoreAndroid");
         initWebViewSettings();
         
@@ -354,7 +356,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:") || internalWhitelist.isUrlWhiteListed(url))
{
+        if (url.startsWith("javascript:") || helper.shouldAllowNavigation(url)) {
             super.loadUrl(url);
         }
     }
@@ -429,7 +431,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);

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/23584274/framework/src/org/apache/cordova/CordovaActivity.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/CordovaActivity.java b/framework/src/org/apache/cordova/CordovaActivity.java
index e065a9b..75ac729 100755
--- a/framework/src/org/apache/cordova/CordovaActivity.java
+++ b/framework/src/org/apache/cordova/CordovaActivity.java
@@ -554,7 +554,11 @@ public class CordovaActivity extends Activity implements CordovaInterface
{
 
         // 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, appView);
+        if ((errorUrl != null) &&
+            (!failingUrl.equals(errorUrl)) &&
+            (appView != null && helper.shouldAllowNavigation(errorUrl))
+           ) {
             // Load URL on UI thread
             me.runOnUiThread(new Runnable() {
                 public void run() {

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/23584274/framework/src/org/apache/cordova/CordovaBridge.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/CordovaBridge.java b/framework/src/org/apache/cordova/CordovaBridge.java
index c3f10f3..c1760e3 100644
--- a/framework/src/org/apache/cordova/CordovaBridge.java
+++ b/framework/src/org/apache/cordova/CordovaBridge.java
@@ -37,12 +37,14 @@ public class CordovaBridge {
     private NativeToJsMessageQueue jsMessageQueue;
     private volatile int expectedBridgeSecret = -1; // written by UI thread, read by JS thread.
     private String loadedUrl;
+    protected CordovaUriHelper helper;
 
-    public CordovaBridge(PluginManager pluginManager, NativeToJsMessageQueue jsMessageQueue)
{
+    public CordovaBridge(PluginManager pluginManager, NativeToJsMessageQueue jsMessageQueue,
CordovaUriHelper helper) {
         this.pluginManager = pluginManager;
         this.jsMessageQueue = jsMessageQueue;
+        this.helper = helper;
     }
-    
+
     public String jsExec(int bridgeSecret, String service, String action, String callbackId,
String arguments) throws JSONException, IllegalAccessException {
         if (!verifySecret("exec()", bridgeSecret)) {
             return null;
@@ -163,9 +165,9 @@ 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 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("http") && loadedUrl.startsWith(origin)))
{
+            // Trust only file URLs and pages which the app would have been allowed
+            // to navigate to anyway.
+            if (origin.startsWith("file:") || helper.shouldAllowNavigation(origin)) {
                 // Enable the bridge
                 int bridgeMode = Integer.parseInt(defaultValue.substring(9));
                 jsMessageQueue.setBridgeMode(bridgeMode);

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/23584274/framework/src/org/apache/cordova/CordovaUriHelper.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/CordovaUriHelper.java b/framework/src/org/apache/cordova/CordovaUriHelper.java
index 6c1c4fa..9aea817 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;
     }
 }

http://git-wip-us.apache.org/repos/asf/cordova-android/blob/23584274/framework/src/org/apache/cordova/IceCreamCordovaWebViewClient.java
----------------------------------------------------------------------
diff --git a/framework/src/org/apache/cordova/IceCreamCordovaWebViewClient.java b/framework/src/org/apache/cordova/IceCreamCordovaWebViewClient.java
index 68f8741..33b7bac 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());
     }


---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@cordova.apache.org
For additional commands, e-mail: commits-help@cordova.apache.org


Mime
View raw message