Files
electron/patches/chromium/fix_crash_loading_non-standard_schemes_in_iframes.patch

97 lines
5.3 KiB
Diff

From 0000000000000000000000000000000000000000 Mon Sep 17 00:00:00 2001
From: Shelley Vohr <shelley.vohr@gmail.com>
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.
Upstream, https://bugs.chromium.org/p/chromium/issues/detail?id=1081397
contains several paths forward - here I chose to swap out the
CHECK in navigation_request.cc from policy->CanAccessDataForOrigin to
policy->CanCommitOriginAndUrl.
Upstreamed at https://chromium-review.googlesource.com/c/chromium/src/+/3856266.
diff --git a/content/browser/renderer_host/navigation_request.cc b/content/browser/renderer_host/navigation_request.cc
index 7bc2f882a1d0d9dfd4541d4da1975e0136cf275e..cc5f926bf73a0d98a0b0015391413867d514f487 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -7547,10 +7547,11 @@ NavigationRequest::GetOriginForURLLoaderFactoryAfterResponseWithDebugInfo() {
if (IsForMhtmlSubframe())
return origin_with_debug_info;
- int process_id = GetRenderFrameHost()->GetProcess()->GetID();
- auto* policy = ChildProcessSecurityPolicyImpl::GetInstance();
- CHECK(
- policy->CanAccessDataForOrigin(process_id, origin_with_debug_info.first));
+ CanCommitStatus can_commit = GetRenderFrameHost()->CanCommitOriginAndUrl(
+ origin_with_debug_info.first, GetURL(), IsSameDocument(), IsPdf(),
+ GetUrlInfo().is_sandboxed);
+ CHECK_EQ(CanCommitStatus::CAN_COMMIT_ORIGIN_AND_URL, can_commit);
+
return origin_with_debug_info;
}
diff --git a/content/browser/renderer_host/render_frame_host_impl.cc b/content/browser/renderer_host/render_frame_host_impl.cc
index 8784a4a93993110879564c2cd5520e2d69f59b07..59bcb9969b014e5df8856f956de1587e2fcf52ec 100644
--- a/content/browser/renderer_host/render_frame_host_impl.cc
+++ b/content/browser/renderer_host/render_frame_host_impl.cc
@@ -10153,9 +10153,11 @@ void RenderFrameHostImpl::CommitNavigation(
ProcessLock::FromSiteInfo(GetSiteInstance()->GetSiteInfo());
auto browser_calc_origin_to_commit =
navigation_request->GetOriginToCommitWithDebugInfo();
+ const CanCommitStatus can_commit_status = policy->CanCommitOriginAndUrl(
+ GetProcess()->GetID(), GetSiteInstance()->GetIsolationContext(),
+ navigation_request->GetUrlInfo());
if (!process_lock.is_error_page() && !is_mhtml_subframe &&
- !policy->CanAccessDataForOrigin(
- GetProcess()->GetID(), browser_calc_origin_to_commit.first.value())) {
+ can_commit_status != CanCommitStatus::CAN_COMMIT_ORIGIN_AND_URL) {
SCOPED_CRASH_KEY_STRING64("CommitNavigation", "lock_url",
process_lock.ToString());
SCOPED_CRASH_KEY_STRING64(
diff --git a/content/browser/renderer_host/render_frame_host_impl.h b/content/browser/renderer_host/render_frame_host_impl.h
index d9923eb1d22e08d1f77fee2d91653476848a2ae0..176ea6690e162c8ed60203e47dbf199fb6c76c22 100644
--- a/content/browser/renderer_host/render_frame_host_impl.h
+++ b/content/browser/renderer_host/render_frame_host_impl.h
@@ -2969,6 +2969,17 @@ class CONTENT_EXPORT RenderFrameHostImpl
// last committed document.
CookieChangeListener::CookieChangeInfo GetCookieChangeInfo();
+ // Returns whether the given origin and URL is allowed to commit in the
+ // current RenderFrameHost. The |url| is used to ensure it matches the origin
+ // in cases where it is applicable. This is a more conservative check than
+ // RenderProcessHost::FilterURL, since it will be used to kill processes that
+ // commit unauthorized origins.
+ CanCommitStatus CanCommitOriginAndUrl(const url::Origin& origin,
+ const GURL& url,
+ bool is_same_document_navigation,
+ bool is_pdf,
+ bool is_sandboxed);
+
// Sets a ResourceCache in the renderer. `this` must be active and there must
// be no pending navigation. `remote` must have the same and process
// isolation policy.
@@ -3392,17 +3403,6 @@ class CONTENT_EXPORT RenderFrameHostImpl
// relevant.
void ResetWaitingState();
- // Returns whether the given origin and URL is allowed to commit in the
- // current RenderFrameHost. The |url| is used to ensure it matches the origin
- // in cases where it is applicable. This is a more conservative check than
- // RenderProcessHost::FilterURL, since it will be used to kill processes that
- // commit unauthorized origins.
- CanCommitStatus CanCommitOriginAndUrl(const url::Origin& origin,
- const GURL& url,
- bool is_same_document_navigation,
- bool is_pdf,
- bool is_sandboxed);
-
// Returns whether a subframe navigation request should be allowed to commit
// to the current RenderFrameHost.
bool CanSubframeCommitOriginAndUrl(NavigationRequest* navigation_request);