fix: crash loading non-standard schemes in iframes (#35517)

* fix: crash loading non-standard schemes in iframes

* test: move fixture to correct location

* chore: update patches

Co-authored-by: Shelley Vohr <shelley.vohr@gmail.com>
Co-authored-by: Charles Kerr <charles@charleskerr.com>
Co-authored-by: PatchUp <73610968+patchup[bot]@users.noreply.github.com>
This commit is contained in:
trop[bot]
2022-09-21 15:25:43 +02:00
committed by GitHub
parent fc1d8a80e6
commit 0ced2338ea
4 changed files with 112 additions and 1 deletions

View File

@@ -121,3 +121,4 @@ fix_revert_emulationhandler_update_functions_to_early_return.patch
fix_return_v8_value_from_localframe_requestexecutescript.patch
disable_optimization_guide_for_preconnect_feature.patch
fix_the_gn_gen_for_components_segmentation_platform.patch
fix_crash_loading_non-standard_schemes_in_iframes.patch

View File

@@ -0,0 +1,78 @@
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 37434a26db44ed035fcbebd9febbda10efa859da..060b310d38db85944e37b8a202493212106d8946 100644
--- a/content/browser/renderer_host/navigation_request.cc
+++ b/content/browser/renderer_host/navigation_request.cc
@@ -6573,10 +6573,11 @@ std::pair<url::Origin, std::string> NavigationRequest::
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.h b/content/browser/renderer_host/render_frame_host_impl.h
index 6aff64db8cc09f95d658fe9e0bd54c0b4c6ff433..e1dda0c951f9ea6f28b6d43ab2b9d4481f5d7773 100644
--- a/content/browser/renderer_host/render_frame_host_impl.h
+++ b/content/browser/renderer_host/render_frame_host_impl.h
@@ -2557,6 +2557,17 @@ class CONTENT_EXPORT RenderFrameHostImpl
HandleAXEvents(tree_id, std::move(updates_and_events), reset_token);
}
+ // 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);
+
protected:
friend class RenderFrameHostFactory;
@@ -2892,17 +2903,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);

View File

@@ -9,7 +9,7 @@ import * as fs from 'fs';
import * as qs from 'querystring';
import * as stream from 'stream';
import { EventEmitter } from 'events';
import { closeWindow } from './window-helpers';
import { closeAllWindows, closeWindow } from './window-helpers';
import { emittedOnce } from './events-helpers';
import { WebmGenerator } from './video-helpers';
import { delay } from './spec-helpers';
@@ -216,6 +216,8 @@ describe('protocol module', () => {
const normalPath = path.join(fixturesPath, 'pages', 'a.html');
const normalContent = fs.readFileSync(normalPath);
afterEach(closeAllWindows);
it('sends file path as response', async () => {
registerFileProtocol(protocolName, (request, callback) => callback(filePath));
const r = await ajax(protocolName + '://fake-host');
@@ -239,6 +241,25 @@ describe('protocol module', () => {
expect(r.headers).to.have.property('x-great-header', 'sogreat');
});
it('can load iframes with custom protocols', (done) => {
registerFileProtocol('custom', (request, callback) => {
const filename = request.url.substring(9);
const p = path.join(__dirname, 'fixtures', 'pages', filename);
callback({ path: p });
});
const w = new BrowserWindow({
show: false,
webPreferences: {
nodeIntegration: true,
contextIsolation: false
}
});
w.loadFile(path.join(__dirname, 'fixtures', 'pages', 'iframe-protocol.html'));
ipcMain.once('loaded-iframe-custom-protocol', () => done());
});
it.skip('throws an error when custom headers are invalid', (done) => {
registerFileProtocol(protocolName, (request, callback) => {
expect(() => callback({

View File

@@ -0,0 +1,11 @@
<body>
<iframe src="custom://base-page.html"></iframe>
<script>
const { ipcRenderer } = require('electron');
const iframe = document.querySelector('iframe');
iframe.addEventListener('load', () => {
ipcRenderer.send('loaded-iframe-custom-protocol');
});
</script>
</body>