Use unsigned Web Bundle claimed URL for window.location and document.URL

Currently the claimed URL of unsigned Web Bundle file is used only for
relative path computation in the document. This is done by setting the
base URL of the document. (https://crrev.com/c/1802902)
  Note: |base_url_override_for_bundled_exchanges| was renamed to
  |base_url_override_for_web_bundle| by https://crrev.com/c/1923786. And
  this CL renames it to |web_bundle_claimed_url|.

But the file path of the Web Bundle file is still available using
window.location and document.URL. This is problematic from the privacy
point of view. This may leak the user name.
  Note: The same problem exists in opening local HTML files
  (https://crbug.com/990216).

To solve this problem, this CL changes window.location and document.URL
to use the claimed URL. This behavior is written in the explainer doc of
“Navigation to Unsigned Web Bundles”.
https://github.com/WICG/webpackage/blob/master/explainers/navigation-to-unsigned-bundles.md#loading-an-untrusted-bundle

Bug: 995177,1023929

Change-Id: I922c4b40504fc1ed87e78cf77c6877b59110dedc
Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/2040500
Reviewed-by: Kinuko Yasuda <kinuko@chromium.org>
Reviewed-by: Kunihiko Sakamoto <ksakamoto@chromium.org>
Commit-Queue: Tsuyoshi Horo <horo@chromium.org>
Cr-Commit-Position: refs/heads/master@{#749646}

content/browser/frame_host/render_frame_host_impl.cc

@@ -5514,9 +5514,9 @@ void RenderFrameHostImpl::CommitNavigation(
       DCHECK(web_bundle_handle_->navigation_info());
       commit_params->web_bundle_physical_url =
           web_bundle_handle_->navigation_info()->source().url();
-      if (web_bundle_handle_->base_url_override().is_valid()) {
-        commit_params->base_url_override_for_web_bundle =
-            web_bundle_handle_->base_url_override();
+      if (web_bundle_handle_->claimed_url().is_valid()) {
+        commit_params->web_bundle_claimed_url =
+            web_bundle_handle_->claimed_url();
       }
     }
 

content/browser/web_package/using_web_bundles.md

@@ -50,6 +50,5 @@ and replace `new Worker(scriptURL)` with `newWorkerViaBlob(scriptURL)`.
 Note that you may also have to fix `importScripts()` or `fetch()` from the worker script that use relative URLs, because the worker’s base URL is now `blob://...`.
 
 ### Other things that do not work in unsigned bundles
-- Scripts using the value of `location.href` may not work (use `document.baseURI`'s value instead).
 - Service workers (does not work in file://)
 - History API (does not work in opaque origins)

content/browser/web_package/web_bundle_browsertest.cc

@@ -556,6 +556,14 @@ class WebBundleTrustableFileBrowserTest
   }
   ~WebBundleTrustableFileBrowserTest() override = default;
 
+  std::string ExecuteAndGetString(const std::string& script) {
+    std::string result;
+    EXPECT_TRUE(content::ExecuteScriptAndExtractString(
+        shell()->web_contents(), "domAutomationController.send(" + script + ")",
+        &result));
+    return result;
+  }
+
  private:
   DISALLOW_COPY_AND_ASSIGN(WebBundleTrustableFileBrowserTest);
 };
@@ -635,6 +643,34 @@ IN_PROC_BROWSER_TEST_P(WebBundleTrustableFileBrowserTest, NavigationWithHash) {
     return;
   NavigateToBundleAndWaitForReady(test_data_url(), GURL(kTestPageUrl));
   NavigateToURLAndWaitForTitle(GURL(kTestPageForHashUrl), "#hello");
+
+  EXPECT_EQ(ExecuteAndGetString("window.location.href"),
+            "https://test.example.org/hash.html#hello");
+  EXPECT_EQ(ExecuteAndGetString("document.location.href"),
+            "https://test.example.org/hash.html#hello");
+  EXPECT_EQ(ExecuteAndGetString("document.URL"),
+            "https://test.example.org/hash.html#hello");
+}
+
+IN_PROC_BROWSER_TEST_P(WebBundleTrustableFileBrowserTest, BaseURI) {
+  // Don't run the test if we couldn't override BrowserClient. It happens only
+  // on Android Kitkat or older systems.
+  if (!original_client_)
+    return;
+  NavigateToBundleAndWaitForReady(test_data_url(), GURL(kTestPageUrl));
+  EXPECT_EQ(ExecuteAndGetString("(new Request('./foo/bar')).url"),
+            "https://test.example.org/foo/bar");
+  EXPECT_EQ(ExecuteAndGetString(R"(
+            (() => {
+              const base_element = document.createElement('base');
+              base_element.href = 'https://example.org/piyo/';
+              document.body.appendChild(base_element);
+              return document.baseURI;
+            })()
+            )"),
+            "https://example.org/piyo/");
+  EXPECT_EQ(ExecuteAndGetString("(new Request('./foo/bar')).url"),
+            "https://example.org/piyo/foo/bar");
 }
 
 INSTANTIATE_TEST_SUITE_P(WebBundleTrustableFileBrowserTests,

content/browser/web_package/web_bundle_handle.cc

@@ -1297,7 +1297,7 @@ void WebBundleHandle::OnWebBundleFileLoaded(
     std::unique_ptr<WebBundleURLLoaderFactory> url_loader_factory) {
   auto source = url_loader_factory->reader()->source().Clone();
   if (source->is_file())
-    base_url_override_ = target_inner_url;
+    claimed_url_ = target_inner_url;
   navigation_info_ = std::make_unique<WebBundleNavigationInfo>(
       std::move(source), target_inner_url,
       url_loader_factory->reader()->GetWeakPtr());

content/browser/web_package/web_bundle_handle.h

@@ -62,9 +62,10 @@ class WebBundleHandle {
   // Checks if a valid Web Bundle is attached, opened, and ready for use.
   bool IsReadyForLoading();
 
-  // The base URL which will be set for the document to support relative path
-  // subresource loading in unsigned Web Bundle file.
-  const GURL& base_url_override() const { return base_url_override_; }
+  // The claimed URL inside Web Bundle file from which the document is loaded.
+  // This URL is used for window.location and document.URL and relative path
+  // computation in the document.
+  const GURL& claimed_url() const { return claimed_url_; }
 
   const WebBundleNavigationInfo* navigation_info() const {
     return navigation_info_.get();
@@ -84,7 +85,7 @@ class WebBundleHandle {
 
   std::unique_ptr<NavigationLoaderInterceptor> interceptor_;
 
-  GURL base_url_override_;
+  GURL claimed_url_;
   std::unique_ptr<WebBundleNavigationInfo> navigation_info_;
 
   std::unique_ptr<WebBundleURLLoaderFactory> url_loader_factory_;

content/common/navigation_params.mojom

@@ -373,9 +373,8 @@ struct CommitNavigationParams {
   // Used as an additional identifier for MemoryCache.
   url.mojom.Url web_bundle_physical_url;
 
-  // The base URL which will be set for the document to support relative path
-  // subresource loading in unsigned Web Bundle file.
-  url.mojom.Url base_url_override_for_web_bundle;
+  // The claimed URL inside Web Bundle from which the document is loaded.
+  url.mojom.Url web_bundle_claimed_url;
 
   // A snapshot value of frame policy(both sandbox flags and container policy)
   // of the frame that is being navigated. The snapshot value is captured at the

content/renderer/render_frame_impl.cc

@@ -509,8 +509,8 @@ void FillNavigationParamsRequest(
   navigation_params->had_transient_activation = common_params.has_user_gesture;
   navigation_params->web_bundle_physical_url =
       commit_params.web_bundle_physical_url;
-  navigation_params->base_url_override_for_web_bundle =
-      commit_params.base_url_override_for_web_bundle;
+  navigation_params->web_bundle_claimed_url =
+      commit_params.web_bundle_claimed_url;
 
   WebVector<WebString> web_origin_trials;
   web_origin_trials.reserve(commit_params.force_enabled_origin_trials.size());

third_party/blink/public/web/web_navigation_params.h

@@ -363,9 +363,10 @@ struct BLINK_EXPORT WebNavigationParams {
   // Used as an additional identifier for MemoryCache.
   WebURL web_bundle_physical_url;
 
-  // The base URL which will be set for the document to support relative path
-  // subresource loading in an unsigned Web Bundle file.
-  WebURL base_url_override_for_web_bundle;
+  // The claimed URL inside Web Bundle file from which the document is loaded.
+  // This URL is used for window.location and document.URL and relative path
+  // computation in the document.
+  WebURL web_bundle_claimed_url;
 
   // The frame policy specified by the frame owner element.
   // Should be base::nullopt for top level navigations

third_party/blink/renderer/core/dom/document.cc

@@ -815,6 +815,11 @@ Document::Document(const DocumentInit& initializer,
     UpdateBaseURL();
   }
 
+  if (initializer.GetWebBundleClaimedUrl().IsValid()) {
+    web_bundle_claimed_url_ = initializer.GetWebBundleClaimedUrl();
+    SetBaseURLOverride(initializer.GetWebBundleClaimedUrl());
+  }
+
   InitSecurityContext(initializer);
   PoliciesInitialized(initializer);
   InitDNSPrefetch();
@@ -4474,6 +4479,9 @@ void Document::writeln(v8::Isolate* isolate,
 }
 
 KURL Document::urlForBinding() const {
+  if (WebBundleClaimedUrl().IsValid()) {
+    return WebBundleClaimedUrl();
+  }
   if (!Url().IsNull()) {
     return Url();
   }

third_party/blink/renderer/core/dom/document.h

@@ -852,6 +852,8 @@ class CORE_EXPORT Document : public ContainerNode,
   KURL CompleteURLWithOverride(const String&,
                                const KURL& base_url_override) const;
 
+  const KURL& WebBundleClaimedUrl() const { return web_bundle_claimed_url_; }
+
   // Determines whether a new document should take on the same origin as that of
   // the document which created it.
   static bool ShouldInheritSecurityOriginFromOwner(const KURL&);
@@ -1981,6 +1983,8 @@ class CORE_EXPORT Document : public ContainerNode,
   KURL cookie_url_;         // The URL to use for cookie access.
   std::unique_ptr<OriginAccessEntry> access_entry_from_url_;
 
+  KURL web_bundle_claimed_url_;
+
   AtomicString base_target_;
 
   // Mime-type of the document in case it was cloned or created by XHR.

third_party/blink/renderer/core/dom/document_init.cc

@@ -422,6 +422,12 @@ DocumentInit& DocumentInit::WithReportOnlyDocumentPolicyHeader(
   return *this;
 }
 
+DocumentInit& DocumentInit::WithWebBundleClaimedUrl(
+    const KURL& web_bundle_claimed_url) {
+  web_bundle_claimed_url_ = web_bundle_claimed_url;
+  return *this;
+}
+
 bool IsPagePopupRunningInWebTest(LocalFrame* frame) {
   return frame && frame->GetPage()->GetChromeClient().IsPopup() &&
          WebTestSupport::IsRunningWebTest();

third_party/blink/renderer/core/dom/document_init.h

@@ -185,6 +185,9 @@ class CORE_EXPORT DocumentInit final {
     return report_only_document_policy_header_;
   }
 
+  DocumentInit& WithWebBundleClaimedUrl(const KURL& web_bundle_claimed_url);
+  const KURL& GetWebBundleClaimedUrl() const { return web_bundle_claimed_url_; }
+
   WindowAgentFactory* GetWindowAgentFactory() const;
   Settings* GetSettingsForWindowAgentFactory() const;
 
@@ -268,6 +271,11 @@ class CORE_EXPORT DocumentInit final {
   DocumentPolicy::FeatureState document_policy_;
   String report_only_document_policy_header_;
 
+  // The claimed URL inside Web Bundle file from which the document is loaded.
+  // This URL is used for window.location and document.URL and relative path
+  // computation in the document.
+  KURL web_bundle_claimed_url_;
+
   bool is_for_external_handler_ = false;
   Color plugin_background_color_;
 };

third_party/blink/renderer/core/frame/location.cc

@@ -56,6 +56,10 @@ void Location::Trace(Visitor* visitor) {
 }
 
 inline const KURL& Location::Url() const {
+  const KURL& web_bundle_claimed_url = GetDocument()->WebBundleClaimedUrl();
+  if (web_bundle_claimed_url.IsValid()) {
+    return web_bundle_claimed_url;
+  }
   const KURL& url = GetDocument()->Url();
   if (!url.IsValid()) {
     // Use "about:blank" while the page is still loading (before we have a

third_party/blink/renderer/core/loader/document_loader.cc

@@ -287,7 +287,7 @@ DocumentLoader::DocumentLoader(
   }
 
   web_bundle_physical_url_ = params_->web_bundle_physical_url;
-  base_url_override_for_web_bundle_ = params_->base_url_override_for_web_bundle;
+  web_bundle_claimed_url_ = params_->web_bundle_claimed_url;
 
   force_enabled_origin_trials_.ReserveInitialCapacity(
       SafeCast<wtf_size_t>(params_->force_enabled_origin_trials.size()));
@@ -780,10 +780,10 @@ void DocumentLoader::HandleRedirect(const KURL& current_request_url) {
   // (eg: file:///tmp/a.wbn), Chrome internally redirects the navigation to the
   // page (eg: https://example.com/page.html) inside the Web Bundle file
   // to the file's URL (file:///tmp/a.wbn?https://example.com/page.html). In
-  // this case, CanDisplay() returns false, and
-  // base_url_override_for_web_bundle must not be null.
+  // this case, CanDisplay() returns false, and web_bundle_claimed_url must not
+  // be null.
   CHECK(SecurityOrigin::Create(current_request_url)->CanDisplay(url_) ||
-        !params_->base_url_override_for_web_bundle.IsNull());
+        !params_->web_bundle_claimed_url.IsNull());
 
   DCHECK(!GetTiming().FetchStart().is_null());
   redirect_chain_.push_back(url_);
@@ -883,10 +883,6 @@ void DocumentLoader::HandleResponse() {
 void DocumentLoader::CommitNavigation() {
   CHECK_GE(state_, kCommitted);
 
-  KURL overriding_url = base_url_override_for_web_bundle_;
-  if (loading_mhtml_archive_ && archive_)
-    overriding_url = archive_->MainResource()->Url();
-
   // Prepare a DocumentInit before clearing the frame, because it may need to
   // inherit an aliased security context.
   Document* owner_document = nullptr;
@@ -907,8 +903,7 @@ void DocumentLoader::CommitNavigation() {
   }
   DCHECK(frame_->GetPage());
 
-  InstallNewDocument(Url(), initiator_origin, owner_document, MimeType(),
-                     overriding_url);
+  InstallNewDocument(Url(), initiator_origin, owner_document, MimeType());
 }
 
 void DocumentLoader::CommitData(const char* bytes, size_t length) {
@@ -1472,8 +1467,7 @@ void DocumentLoader::InstallNewDocument(
     const KURL& url,
     const scoped_refptr<const SecurityOrigin> initiator_origin,
     Document* owner_document,
-    const AtomicString& mime_type,
-    const KURL& overriding_url) {
+    const AtomicString& mime_type) {
   DCHECK(!frame_->GetDocument() || !frame_->GetDocument()->IsActive());
   DCHECK_EQ(frame_->Tree().ChildCount(), 0u);
 
@@ -1527,7 +1521,8 @@ void DocumentLoader::InstallNewDocument(
               response_.HttpHeaderField(http_names::kDocumentPolicyReportOnly))
           .WithOriginTrialsHeader(
               response_.HttpHeaderField(http_names::kOriginTrial))
-          .WithContentSecurityPolicy(content_security_policy_.Get());
+          .WithContentSecurityPolicy(content_security_policy_.Get())
+          .WithWebBundleClaimedUrl(web_bundle_claimed_url_);
 
   // A javascript: url inherits CSP from the document in which it was
   // executed.
@@ -1610,8 +1605,11 @@ void DocumentLoader::InstallNewDocument(
     frame_->Tree().ExperimentalSetNulledName();
   }
 
-  if (!overriding_url.IsEmpty())
-    document->SetBaseURLOverride(overriding_url);
+  if (loading_mhtml_archive_ && archive_ &&
+      !archive_->MainResource()->Url().IsEmpty()) {
+    document->SetBaseURLOverride(archive_->MainResource()->Url());
+  }
+
   DidInstallNewDocument(document);
 
   // This must be called before the document is opened, otherwise HTML parser

third_party/blink/renderer/core/loader/document_loader.h

@@ -343,8 +343,7 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
       const KURL&,
       const scoped_refptr<const SecurityOrigin> initiator_origin,
       Document* owner_document,
-      const AtomicString& mime_type,
-      const KURL& overriding_url);
+      const AtomicString& mime_type);
   void DidInstallNewDocument(Document*);
   void WillCommitNavigation();
   void DidCommitNavigation();
@@ -533,7 +532,7 @@ class CORE_EXPORT DocumentLoader : public GarbageCollected<DocumentLoader>,
   Member<SourceKeyedCachedMetadataHandler> cached_metadata_handler_;
   Member<PrefetchedSignedExchangeManager> prefetched_signed_exchange_manager_;
   KURL web_bundle_physical_url_;
-  KURL base_url_override_for_web_bundle_;
+  KURL web_bundle_claimed_url_;
 
   // This UseCounterHelper tracks feature usage associated with the lifetime of
   // the document load. Features recorded prior to commit will be recorded