fix: crash loading non-standard schemes in iframes (#35485)
This commit is contained in:
parent
bfced8cbfe
commit
e0fb5cbe1f
4 changed files with 112 additions and 1 deletions
|
@ -118,3 +118,4 @@ revert_spellcheck_fully_launch_spell_check_delayed_initialization.patch
|
||||||
add_electron_deps_to_license_credits_file.patch
|
add_electron_deps_to_license_credits_file.patch
|
||||||
feat_add_set_can_resize_mutator.patch
|
feat_add_set_can_resize_mutator.patch
|
||||||
fix_revert_emulationhandler_update_functions_to_early_return.patch
|
fix_revert_emulationhandler_update_functions_to_early_return.patch
|
||||||
|
fix_crash_loading_non-standard_schemes_in_iframes.patch
|
||||||
|
|
|
@ -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 b1120c9270a255e4b6070c0d2b5f49e6f99d935d..407888cb17d3e609ad7aaa2ed1c079642df60bca 100644
|
||||||
|
--- a/content/browser/renderer_host/navigation_request.cc
|
||||||
|
+++ b/content/browser/renderer_host/navigation_request.cc
|
||||||
|
@@ -6517,10 +6517,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 b1259618ff4dc1848cf007d853e6f3e70b08829f..3d4e6c4c7e0348c52c3f0ccd0f6c19e0e1ec15a5 100644
|
||||||
|
--- a/content/browser/renderer_host/render_frame_host_impl.h
|
||||||
|
+++ b/content/browser/renderer_host/render_frame_host_impl.h
|
||||||
|
@@ -2550,6 +2550,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;
|
||||||
|
|
||||||
|
@@ -2879,17 +2890,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);
|
|
@ -9,7 +9,7 @@ import * as fs from 'fs';
|
||||||
import * as qs from 'querystring';
|
import * as qs from 'querystring';
|
||||||
import * as stream from 'stream';
|
import * as stream from 'stream';
|
||||||
import { EventEmitter } from 'events';
|
import { EventEmitter } from 'events';
|
||||||
import { closeWindow } from './window-helpers';
|
import { closeAllWindows, closeWindow } from './window-helpers';
|
||||||
import { emittedOnce } from './events-helpers';
|
import { emittedOnce } from './events-helpers';
|
||||||
import { WebmGenerator } from './video-helpers';
|
import { WebmGenerator } from './video-helpers';
|
||||||
import { delay } from './spec-helpers';
|
import { delay } from './spec-helpers';
|
||||||
|
@ -216,6 +216,8 @@ describe('protocol module', () => {
|
||||||
const normalPath = path.join(fixturesPath, 'pages', 'a.html');
|
const normalPath = path.join(fixturesPath, 'pages', 'a.html');
|
||||||
const normalContent = fs.readFileSync(normalPath);
|
const normalContent = fs.readFileSync(normalPath);
|
||||||
|
|
||||||
|
afterEach(closeAllWindows);
|
||||||
|
|
||||||
it('sends file path as response', async () => {
|
it('sends file path as response', async () => {
|
||||||
registerFileProtocol(protocolName, (request, callback) => callback(filePath));
|
registerFileProtocol(protocolName, (request, callback) => callback(filePath));
|
||||||
const r = await ajax(protocolName + '://fake-host');
|
const r = await ajax(protocolName + '://fake-host');
|
||||||
|
@ -239,6 +241,25 @@ describe('protocol module', () => {
|
||||||
expect(r.headers).to.have.property('x-great-header', 'sogreat');
|
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) => {
|
it.skip('throws an error when custom headers are invalid', (done) => {
|
||||||
registerFileProtocol(protocolName, (request, callback) => {
|
registerFileProtocol(protocolName, (request, callback) => {
|
||||||
expect(() => callback({
|
expect(() => callback({
|
||||||
|
|
11
spec/fixtures/pages/iframe-protocol.html
vendored
Normal file
11
spec/fixtures/pages/iframe-protocol.html
vendored
Normal 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>
|
Loading…
Add table
Add a link
Reference in a new issue