From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001 From: Shelley Vohr Date: Mon, 29 Aug 2022 11:44:57 +0200 Subject: fix: crash loading non-standard schemes in iframes This fixes a crash that occurs when loading non-standard schemes from iframes or webviews. This was happening because ChildProcessSecurityPolicyImpl::CanAccessDataForOrigin contains explicit exceptions to allow built-in non-standard schemes, but does not check for non-standard schemes registered by the embedder. This patch adjusts the origin calculation for subframe non-standard schemes in - browser process at `NavigationRequest::GetOriginForURLLoaderFactoryUncheckedWithDebugInfo` - render process at `DocumentLoader::CalculateOrigin` When top level frame navigates to non-standard scheme url, the origin is calculated as `null` without any derivation. It is only in cases where there is a `initiator_origin` then the origin is derived from it, which is usually the case for renderer initiated navigations and iframes are no exceptions from this rule. The patch should be removed in favor of either: - Remove support for non-standard custom schemes - Register non-standard custom schemes as websafe schemes and update CPSPI::CanAccessDataForOrigin to allow them for any navigation. - Update the callsite to use RFHI::CanCommitOriginAndUrl in upstream, previous effort to do this can be found at https://chromium-review.googlesource.com/c/chromium/src/+/3856266. Upstream bug https://bugs.chromium.org/p/chromium/issues/detail?id=1081397. diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc index 0c67607fd99b2fceba176308a041c8f08643506a..82c4a7e1d441f1444d6ca32a56e8b0381209ec2f 100644 --- a/content/browser/renderer_host/navigation_request.cc +++ b/content/browser/renderer_host/navigation_request.cc @@ -10980,6 +10980,12 @@ NavigationRequest::GetOriginForURLLoaderFactoryUncheckedWithDebugInfo() { "blob"); } + if (!IsInMainFrame() && !common_params().url.IsStandard()) { + return std::make_pair(url::Origin::Resolve(common_params().url, + url::Origin()), + "url_non_standard"); + } + // In cases not covered above, URLLoaderFactory should be associated with the // origin of |common_params.url| and/or |common_params.initiator_origin|. url::Origin resolved_origin = url::Origin::Resolve( diff --git a/third_party/blink/renderer/core/loader/document_loader.cc b/third_party/blink/renderer/core/loader/document_loader.cc index cbf8b2ed5ce03a3b4524f4e7f97bbb7750e5c583..b84bdff826390234f470025edba02abddefff045 100644 --- a/third_party/blink/renderer/core/loader/document_loader.cc +++ b/third_party/blink/renderer/core/loader/document_loader.cc @@ -2325,6 +2325,10 @@ Frame* DocumentLoader::CalculateOwnerFrame() { scoped_refptr DocumentLoader::CalculateOrigin( Document* owner_document) { scoped_refptr origin; + bool is_standard = false; + std::string protocol = url_.Protocol().Ascii(); + is_standard = url::IsStandard( + protocol.data(), url::Component(0, static_cast(protocol.size()))); StringBuilder debug_info_builder; // Whether the origin is newly created within this call, instead of copied // from an existing document's origin or from `origin_to_commit_`. If this is @@ -2378,6 +2382,10 @@ scoped_refptr DocumentLoader::CalculateOrigin( // the end of this function. origin = origin_to_commit_; debug_info_builder.Append("use_origin_to_commit"); + } else if (!SecurityOrigin::ShouldUseInnerURL(url_) && + !is_standard) { + debug_info_builder.Append("use_url_with_non_standard_scheme"); + origin = SecurityOrigin::Create(url_); } else { debug_info_builder.Append("use_url_with_precursor"); // Otherwise, create an origin that propagates precursor information