Restrict webview.loadDataWithBaseUrl base URL

There were no restrictions on which URLs could be used as the base URL
when using webview's loadDataWithBaseUrl API. This could allow for an
embedder to impersonate another extension through a webview.

We now restrict the base URL to HTTP(S) or the embedder's own origin.

Bug: 1108126
Change-Id: I093a3d2c75cfb2f307ceca43add513194df13854
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2553704Reviewed-by: James MacLean <wjmaclean@chromium.org>
Commit-Queue: Kevin McNee <mcnee@chromium.org>
Cr-Commit-Position: refs/heads/master@{#831054}

chrome/browser/apps/guest_view/web_view_browsertest.cc

@@ -21,6 +21,7 @@
 #include "base/strings/stringprintf.h"
 #include "base/strings/utf_string_conversions.h"
 #include "base/test/test_mock_time_task_runner.h"
+#include "base/test/test_timeouts.h"
 #include "base/threading/thread_restrictions.h"
 #include "base/threading/thread_task_runner_handle.h"
 #include "build/build_config.h"
@@ -62,6 +63,7 @@
 #include "content/public/browser/ax_event_notification_details.h"
 #include "content/public/browser/browser_thread.h"
 #include "content/public/browser/gpu_data_manager.h"
+#include "content/public/browser/navigation_handle.h"
 #include "content/public/browser/notification_service.h"
 #include "content/public/browser/render_frame_host.h"
 #include "content/public/browser/render_process_host.h"
@@ -3231,6 +3233,51 @@ IN_PROC_BROWSER_TEST_F(WebViewTest, Shim_TestLoadDataAPI) {
   TestHelper("testLoadDataAPI", "web_view/shim", NEEDS_TEST_SERVER);
 }
 
+IN_PROC_BROWSER_TEST_F(WebViewTest, Shim_TestLoadDataAPIAccessibleResources) {
+  TestHelper("testLoadDataAPIAccessibleResources", "web_view/shim",
+             NEEDS_TEST_SERVER);
+}
+
+namespace {
+// Fails the test if a navigation is started in the given WebContents.
+class FailOnNavigation : public content::WebContentsObserver {
+ public:
+  explicit FailOnNavigation(content::WebContents* contents)
+      : content::WebContentsObserver(contents) {}
+
+  // content::WebContentsObserver:
+  void DidStartNavigation(
+      content::NavigationHandle* navigation_handle) override {
+    ADD_FAILURE() << "Unexpected navigation: " << navigation_handle->GetURL();
+  }
+};
+}  // namespace
+
+IN_PROC_BROWSER_TEST_F(WebViewTest, LoadDataAPINotRelativeToAnotherExtension) {
+  ASSERT_TRUE(StartEmbeddedTestServer());
+  const extensions::Extension* other_extension =
+      LoadExtension(test_data_dir_.AppendASCII("simple_with_file"));
+  LoadAppWithGuest("web_view/simple");
+  content::WebContents* embedder = GetEmbedderWebContents();
+  content::WebContents* guest = GetGuestWebContents();
+
+  FailOnNavigation fail_if_webview_navigates(guest);
+  ASSERT_TRUE(content::ExecuteScript(
+      embedder, content::JsReplace(
+                    "var webview = document.querySelector('webview'); "
+                    "webview.loadDataWithBaseUrl('data:text/html,hello', $1);",
+                    other_extension->url())));
+
+  // We expect the call to loadDataWithBaseUrl to fail and not cause a
+  // navigation. Since loadDataWithBaseUrl doesn't notify when it fails, we
+  // resort to a timeout here. If |fail_if_webview_navigates| doesn't see a
+  // navigation in that time, we consider the test to have passed.
+  base::RunLoop run_loop;
+  base::ThreadTaskRunnerHandle::Get()->PostDelayedTask(
+      FROM_HERE, run_loop.QuitClosure(), TestTimeouts::tiny_timeout());
+  run_loop.Run();
+}
+
 // This test verifies that the resize and contentResize events work correctly.
 IN_PROC_BROWSER_TEST_F(WebViewSizeTest, Shim_TestResizeEvents) {
   TestHelper("testResizeEvents", "web_view/shim", NO_TEST_SERVER);

chrome/test/data/extensions/platform_apps/web_view/shim/main.js

@@ -2854,7 +2854,41 @@ function testLoadDataAPI() {
 
   webview.addEventListener('loadstop', loadstopListener1);
   document.body.appendChild(webview);
-};
+}
+
+// loadDataWithBaseUrl cannot generally be used with a chrome-extension:// base
+// URL, however the embedding extension may use its own chrome-extension://
+// origin. We test that an embedder can use its own origin as the base and that
+// relative URLs resolve to it by loading something in the guest from the
+// embedder's accessible_resources.
+function testLoadDataAPIAccessibleResources() {
+  let webview = new WebView();
+  // The accessible_resources listed in the manifest file are under the
+  // "foobar" partition.
+  webview.partition = 'foobar';
+  webview.src = 'about:blank';
+
+  let loadstopListener2 = function() {
+    webview.executeScript(
+        {code: 'document.querySelector(\'img\').src'}, (e) => {
+          embedder.test.assertEq(e, location.origin + '/test.bmp');
+          embedder.test.succeed();
+        });
+  };
+
+  let loadstopListener1 = function() {
+    webview.removeEventListener('loadstop', loadstopListener1);
+    webview.addEventListener('loadstop', loadstopListener2);
+
+    let encodedData =
+        window.btoa('<html>This is a test.<br><img src="test.bmp"><br></html>');
+    webview.loadDataWithBaseUrl('data:text/html;base64,' + encodedData,
+                                location.origin);
+  };
+
+  webview.addEventListener('loadstop', loadstopListener1);
+  document.body.appendChild(webview);
+}
 
 // Test that the resize events fire with the correct values, and in the
 // correct order, when resizing occurs.
@@ -3417,6 +3451,7 @@ embedder.test.testList = {
   'testFindAPI_findupdate': testFindAPI_findupdate,
   'testFindInMultipleWebViews': testFindInMultipleWebViews,
   'testLoadDataAPI': testLoadDataAPI,
+  'testLoadDataAPIAccessibleResources': testLoadDataAPIAccessibleResources,
   'testResizeEvents': testResizeEvents,
   'testPerOriginZoomMode': testPerOriginZoomMode,
   'testPerViewZoomMode': testPerViewZoomMode,

chrome/test/data/extensions/platform_apps/web_view/shim/manifest.json

@@ -18,6 +18,7 @@
           "child_frame.html",
           "parent_frame.html",
           "guest_with_inline_script.html",
+          "test.bmp",
           "test.pdf",
           "guest_with_select.html"
         ]

extensions/browser/api/guest_view/web_view/web_view_internal_api.cc

@@ -796,8 +796,9 @@ WebViewInternalLoadDataWithBaseUrlFunction::Run() {
       params->virtual_url ? *params->virtual_url : params->data_url;
 
   std::string error;
-  bool successful = guest_->LoadDataWithBaseURL(
-      params->data_url, params->base_url, virtual_url, &error);
+  bool successful = guest_->LoadDataWithBaseURL(GURL(params->data_url),
+                                                GURL(params->base_url),
+                                                GURL(virtual_url), &error);
   if (successful)
     return RespondNow(NoArguments());
   return RespondNow(Error(std::move(error)));

extensions/browser/guest_view/web_view/web_view_guest.cc

@@ -1352,40 +1352,41 @@ void WebViewGuest::SetAllowScaling(bool allow) {
   allow_scaling_ = allow;
 }
 
-bool WebViewGuest::LoadDataWithBaseURL(const std::string& data_url,
-                                       const std::string& base_url,
-                                       const std::string& virtual_url,
+bool WebViewGuest::LoadDataWithBaseURL(const GURL& data_url,
+                                       const GURL& base_url,
+                                       const GURL& virtual_url,
                                        std::string* error) {
-  // Make GURLs from URLs.
-  const GURL data_gurl = GURL(data_url);
-  const GURL base_gurl = GURL(base_url);
-  const GURL virtual_gurl = GURL(virtual_url);
-
   // Check that the provided URLs are valid.
   // |data_url| must be a valid data URL.
-  if (!data_gurl.is_valid() || !data_gurl.SchemeIs(url::kDataScheme)) {
-    base::SStringPrintf(
-        error, webview::kAPILoadDataInvalidDataURL, data_url.c_str());
+  if (!data_url.is_valid() || !data_url.SchemeIs(url::kDataScheme)) {
+    base::SStringPrintf(error, webview::kAPILoadDataInvalidDataURL,
+                        data_url.possibly_invalid_spec().c_str());
     return false;
   }
-  // |base_url| must be a valid URL.
-  if (!base_gurl.is_valid()) {
-    base::SStringPrintf(
-        error, webview::kAPILoadDataInvalidBaseURL, base_url.c_str());
+  const url::Origin& owner_origin =
+      owner_web_contents()->GetMainFrame()->GetLastCommittedOrigin();
+  const bool base_in_owner_origin =
+      owner_origin.IsSameOriginWith(url::Origin::Create(base_url));
+  // |base_url| must be a valid URL. It is also limited to URLs that the owner
+  // is trusted to have control over.
+  if (!base_url.is_valid() ||
+      (!base_url.SchemeIsHTTPOrHTTPS() && !base_in_owner_origin)) {
+    base::SStringPrintf(error, webview::kAPILoadDataInvalidBaseURL,
+                        base_url.possibly_invalid_spec().c_str());
     return false;
   }
   // |virtual_url| must be a valid URL.
-  if (!virtual_gurl.is_valid()) {
-    base::SStringPrintf(
-        error, webview::kAPILoadDataInvalidVirtualURL, virtual_url.c_str());
+  if (!virtual_url.is_valid()) {
+    base::SStringPrintf(error, webview::kAPILoadDataInvalidVirtualURL,
+                        virtual_url.possibly_invalid_spec().c_str());
     return false;
   }
 
   // Set up the parameters to load |data_url| with the specified |base_url|.
-  content::NavigationController::LoadURLParams load_params(data_gurl);
+  content::NavigationController::LoadURLParams load_params(data_url);
   load_params.load_type = content::NavigationController::LOAD_TYPE_DATA;
-  load_params.base_url_for_data_url = base_gurl;
-  load_params.virtual_url_for_data_url = virtual_gurl;
+  load_params.base_url_for_data_url = base_url;
+  load_params.virtual_url_for_data_url = virtual_url;
   load_params.override_user_agent =
       content::NavigationController::UA_OVERRIDE_INHERIT;
 

extensions/browser/guest_view/web_view/web_view_guest.h

@@ -110,9 +110,9 @@ class WebViewGuest : public guest_view::GuestView<WebViewGuest> {
   bool allow_transparency() const { return allow_transparency_; }
 
   // Loads a data URL with a specified base URL and virtual URL.
-  bool LoadDataWithBaseURL(const std::string& data_url,
-                           const std::string& base_url,
-                           const std::string& virtual_url,
+  bool LoadDataWithBaseURL(const GURL& data_url,
+                           const GURL& base_url,
+                           const GURL& virtual_url,
                            std::string* error);
 
   // Begin or continue a find request.