diff --git a/shell/browser/net/electron_url_loader_factory.cc b/shell/browser/net/electron_url_loader_factory.cc index 331e3c376c..5458587903 100644 --- a/shell/browser/net/electron_url_loader_factory.cc +++ b/shell/browser/net/electron_url_loader_factory.cc @@ -4,6 +4,7 @@ #include "shell/browser/net/electron_url_loader_factory.h" +#include #include #include #include @@ -26,11 +27,15 @@ #include "net/http/http_status_code.h" #include "net/http/http_util.h" #include "net/url_request/redirect_util.h" +#include "services/network/public/cpp/cors/cors.h" +#include "services/network/public/cpp/cors/cors_error_status.h" #include "services/network/public/cpp/resource_request.h" #include "services/network/public/cpp/shared_url_loader_factory.h" #include "services/network/public/cpp/simple_url_loader.h" #include "services/network/public/cpp/simple_url_loader_stream_consumer.h" #include "services/network/public/cpp/url_loader_completion_status.h" +#include "services/network/public/mojom/cors.mojom.h" +#include "services/network/public/mojom/fetch_api.mojom.h" #include "services/network/public/mojom/url_loader.mojom.h" #include "services/network/public/mojom/url_loader_factory.mojom.h" #include "services/network/public/mojom/url_response_head.mojom.h" @@ -46,6 +51,7 @@ #include "shell/common/gin_helper/dictionary.h" #include "third_party/abseil-cpp/absl/strings/str_format.h" #include "third_party/blink/public/mojom/loader/resource_load_info.mojom-shared.h" +#include "url/url_util.h" #include "shell/common/node_includes.h" @@ -422,6 +428,26 @@ void ElectronURLLoaderFactory::CreateLoaderAndStart( const net::MutableNetworkTrafficAnnotationTag& traffic_annotation) { DCHECK_CURRENTLY_ON(content::BrowserThread::UI); + // Subresource requests for registered protocols reach this factory via the + // renderer's per-scheme URLLoaderFactoryBundle entry, which bypasses the + // network service's CorsURLLoaderFactory entirely. Replicate the + // kCorsDisabledScheme gate from CorsURLLoader::StartRequest so a cross-origin + // page cannot read responses from a scheme registered with + // {supportFetchAPI: true} but without {corsEnabled: true}. Browser-initiated + // requests (no |request_initiator|) are trusted and skipped. + if (request.request_initiator && + network::cors::ShouldCheckCors(request.url, request.request_initiator, + request.mode) && + !std::ranges::contains(url::GetCorsEnabledSchemes(), + request.url.GetScheme())) { + mojo::Remote client_remote( + std::move(client)); + client_remote->OnComplete( + network::URLLoaderCompletionStatus(network::CorsErrorStatus( + network::mojom::CorsError::kCorsDisabledScheme))); + return; + } + // |StartLoading| is used for both intercepted and registered protocols, // and on redirects it needs a factory to use to create a loader for the // new request. So in this case, this factory is the target factory. @@ -482,6 +508,16 @@ void ElectronURLLoaderFactory::StartLoading( network::mojom::URLResponseHeadPtr head = ToResponseHead(dict); + // For cross-origin no-cors loads (e.g. , fetch({mode:'no-cors'})), the + // body must not be script-readable; tag the response as opaque so Blink + // applies opaque filtering. CorsURLLoader normally does this, but per-scheme + // factories bypass it. + if (request.mode == network::mojom::RequestMode::kNoCors && + request.request_initiator && + !request.request_initiator->IsSameOriginWith(request.url)) { + head->response_type = network::mojom::FetchResponseType::kOpaque; + } + // Handle redirection. // // Note that with NetworkService, sending the "Location" header no longer diff --git a/spec/api-protocol-spec.ts b/spec/api-protocol-spec.ts index 9702bb5be3..693c359387 100644 --- a/spec/api-protocol-spec.ts +++ b/spec/api-protocol-spec.ts @@ -34,7 +34,7 @@ const unregisterProtocol = protocol.unregisterProtocol; const uninterceptProtocol = protocol.uninterceptProtocol; const text = 'valar morghulis'; -const protocolName = 'no-cors'; +const protocolName = 'cors'; const postData = { name: 'post test', type: 'string' @@ -924,7 +924,149 @@ describe('protocol module', () => { }); }); - // DISABLED-FIXME: Figure out why this test is failing + // A scheme registered with only {supportFetchAPI: true} (no + // {corsEnabled: true}) must not be readable cross-origin. + describe('cross-origin enforcement for supportFetchAPI-only schemes', () => { + const secret = 'secret-token-9d4f2c'; + let remoteUrl: string; + let handlerCalls: string[]; + + beforeEach(async () => { + handlerCalls = []; + protocol.handle('no-cors', (req) => { + handlerCalls.push(req.url); + return new Response(secret, { headers: { 'content-type': 'text/plain' } }); + }); + protocol.handle('no-cors-standard', (req) => { + handlerCalls.push(req.url); + if (new URL(req.url).pathname === '/page') { + return new Response('page', { + headers: { 'content-type': 'text/html' } + }); + } + return new Response(secret, { headers: { 'content-type': 'text/plain' } }); + }); + const server = http.createServer((req, res) => { + res.setHeader('content-type', 'text/html'); + res.end('remote'); + }); + defer(() => server.close()); + ({ url: remoteUrl } = await listen(server)); + }); + + afterEach(() => { + protocol.unhandle('no-cors'); + protocol.unhandle('no-cors-standard'); + }); + + it('blocks a remote http origin from reading the response body via fetch()', async () => { + await w.loadURL(remoteUrl); + const consoleMessages: string[] = []; + w.webContents.on('console-message', (e) => consoleMessages.push(e.message)); + const { body, error } = await w.webContents.executeJavaScript(` + fetch('no-cors://host/secret') + .then(r => r.text()).then(body => ({ body, error: null })) + .catch(e => ({ body: null, error: String(e) })) + `); + expect(body).to.not.equal(secret, 'http origin read no-cors:// body via fetch()'); + expect(error) + .to.be.a('string') + .and.match(/Failed to fetch/); + expect(consoleMessages.join('\n')).to.match(/has been blocked by CORS policy/); + }); + + it('blocks a remote http origin from reading the response body via XHR', async () => { + await w.loadURL(remoteUrl); + const { body, errored } = await w.webContents.executeJavaScript(` + new Promise(resolve => { + const x = new XMLHttpRequest(); + x.onload = () => resolve({ body: x.responseText, errored: false }); + x.onerror = () => resolve({ body: null, errored: true }); + x.open('GET', 'no-cors://host/secret'); + x.send(); + }) + `); + expect(body).to.not.equal(secret, 'http origin read no-cors:// body via XHR'); + expect(errored).to.be.true(); + }); + + it('does not invoke the protocol handler for a blocked cross-origin CORS request', async () => { + await w.loadURL(remoteUrl); + await w.webContents.executeJavaScript(` + fetch('no-cors://host/secret', { + method: 'PUT', + headers: { 'x-custom': '1' }, + body: 'x' + }).catch(() => 0) + `); + expect(handlerCalls).to.deep.equal([]); + }); + + it('returns an opaque response for cross-origin fetch with mode: no-cors', async () => { + await w.loadURL(remoteUrl); + const { type, status, body } = await w.webContents.executeJavaScript(` + fetch('no-cors://host/secret', { mode: 'no-cors' }) + .then(async r => ({ type: r.type, status: r.status, body: await r.text() })) + `); + expect(type).to.equal('opaque'); + expect(status).to.equal(0); + expect(body).to.equal(''); + }); + + it('still allows cross-origin loads (no-cors subresource)', async () => { + protocol.unhandle('no-cors'); + protocol.handle( + 'no-cors', + () => + new Response(fs.readFileSync(path.join(fixturesPath, 'assets', 'logo.png')), { + headers: { 'content-type': 'image/png' } + }) + ); + await w.loadURL(remoteUrl); + const { ok, width } = await w.webContents.executeJavaScript(` + new Promise(resolve => { + const img = new Image(); + img.onload = () => resolve({ ok: true, width: img.naturalWidth }); + img.onerror = () => resolve({ ok: false, width: 0 }); + img.src = 'no-cors://host/logo.png'; + }) + `); + expect(ok).to.be.true(); + expect(width).to.be.greaterThan(0); + }); + + it('allows same-origin fetch on a standard supportFetchAPI-only scheme', async () => { + await w.loadURL('no-cors-standard://app/page'); + const body = await w.webContents.executeJavaScript("fetch('no-cors-standard://app/data').then(r => r.text())"); + expect(body).to.equal(secret); + }); + + it('blocks cross-origin fetch on a standard supportFetchAPI-only scheme', async () => { + await w.loadURL('no-cors-standard://app/page'); + handlerCalls = []; + const error = await w.webContents.executeJavaScript( + "fetch('no-cors-standard://other/data').then(() => null, e => String(e))" + ); + expect(error) + .to.be.a('string') + .and.match(/Failed to fetch/); + expect(handlerCalls).to.deep.equal([]); + }); + + it('does not affect cross-origin fetch to a corsEnabled scheme', async () => { + protocol.handle('cors', () => new Response('ok')); + defer(() => protocol.unhandle('cors')); + await w.loadURL(remoteUrl); + const body = await w.webContents.executeJavaScript("fetch('cors://host/').then(r => r.text())"); + expect(body).to.equal('ok'); + }); + + it('does not affect main-process net.fetch', async () => { + const body = await net.fetch('no-cors://host/secret').then((r) => r.text()); + expect(body).to.equal(secret); + }); + }); + it('disallows CORS and fetch requests when only supportFetchAPI is specified', async () => { await allowsCORSRequests('no-cors', ['failed xhr', 'failed fetch'], /has been blocked by CORS policy/, () => { const { ipcRenderer } = require('electron'); @@ -1489,9 +1631,9 @@ describe('protocol module', () => { }); it('can receive streaming fetch upload', async () => { - protocol.handle('no-cors', (req) => new Response(req.body)); - defer(() => { protocol.unhandle('no-cors'); }); - await contents.loadURL('no-cors://foo/'); + protocol.handle('cors', (req) => new Response(req.body)); + defer(() => { protocol.unhandle('cors'); }); + await contents.loadURL('cors://foo/'); const fetchBodyResult = await contents.executeJavaScript(` const stream = new ReadableStream({ async start(controller) { @@ -1513,12 +1655,12 @@ describe('protocol module', () => { session.defaultSession.webRequest.onBeforeRequest(null); }); - protocol.handle('no-cors', (req) => { + protocol.handle('cors', (req) => { console.log('handle', req.url, req.method); return new Response(req.body); }); - defer(() => { protocol.unhandle('no-cors'); }); - await contents.loadURL('no-cors://foo/'); + defer(() => { protocol.unhandle('cors'); }); + await contents.loadURL('cors://foo/'); const fetchBodyResult = await contents.executeJavaScript(` const stream = new ReadableStream({ async start(controller) { @@ -1532,9 +1674,9 @@ describe('protocol module', () => { }); it('can receive an error from streaming fetch upload', async () => { - protocol.handle('no-cors', (req) => new Response(req.body)); - defer(() => { protocol.unhandle('no-cors'); }); - await contents.loadURL('no-cors://foo/'); + protocol.handle('cors', (req) => new Response(req.body)); + defer(() => { protocol.unhandle('cors'); }); + await contents.loadURL('cors://foo/'); const fetchBodyResult = await contents.executeJavaScript(` const stream = new ReadableStream({ async start(controller) { @@ -1549,12 +1691,12 @@ describe('protocol module', () => { it('gets an error from streaming fetch upload when the renderer dies', async () => { let gotRequest: Function; const receivedRequest = new Promise(resolve => { gotRequest = resolve; }); - protocol.handle('no-cors', (req) => { + protocol.handle('cors', (req) => { if (/fetch/.test(req.url)) gotRequest(req); return new Response(); }); - defer(() => { protocol.unhandle('no-cors'); }); - await contents.loadURL('no-cors://foo/'); + defer(() => { protocol.unhandle('cors'); }); + await contents.loadURL('cors://foo/'); contents.executeJavaScript(` const stream = new ReadableStream({ async start(controller) { diff --git a/spec/api-web-request-spec.ts b/spec/api-web-request-spec.ts index 6160129cfa..13146d581c 100644 --- a/spec/api-web-request-spec.ts +++ b/spec/api-web-request-spec.ts @@ -356,12 +356,12 @@ describe('webRequest module', () => { }); it('can change the request headers on a custom protocol redirect', async () => { - protocol.registerStringProtocol('no-cors', (req, callback) => { - if (req.url === 'no-cors://fake-host/redirect') { + protocol.registerStringProtocol('cors-blob', (req, callback) => { + if (req.url === 'cors-blob://fake-host/redirect') { callback({ statusCode: 302, headers: { - Location: 'no-cors://fake-host' + Location: 'cors-blob://fake-host' } }); } else { @@ -384,10 +384,10 @@ describe('webRequest module', () => { requestHeaders.Accept = '*/*;test/header'; callback({ requestHeaders }); }); - const { data } = await ajax('no-cors://fake-host/redirect'); + const { data } = await ajax('cors-blob://fake-host/redirect'); expect(data).to.equal('header-received'); } finally { - protocol.unregisterProtocol('no-cors'); + protocol.unregisterProtocol('cors-blob'); } }); diff --git a/spec/disabled-tests.json b/spec/disabled-tests.json index c2abef4797..e8764dde21 100644 --- a/spec/disabled-tests.json +++ b/spec/disabled-tests.json @@ -3,7 +3,6 @@ "BrowserWindow module BrowserWindow.loadURL(url) should emit did-fail-load event for files that do not exist", "Menu module Menu.setApplicationMenu unsets a menu with null", "process module main process process.takeHeapSnapshot() returns true on success", - "protocol module protocol.registerSchemesAsPrivileged cors-fetch disallows CORS and fetch requests when only supportFetchAPI is specified", "session module ses.cookies should set cookie for standard scheme", "webFrameMain module WebFrame.visibilityState should match window state", "reporting api sends a report for a deprecation", diff --git a/spec/index.js b/spec/index.js index 635ca458dd..629ac8f917 100644 --- a/spec/index.js +++ b/spec/index.js @@ -55,6 +55,7 @@ protocol.registerSchemesAsPrivileged([ { scheme: 'cors-blob', privileges: { corsEnabled: true, supportFetchAPI: true } }, { scheme: 'cors', privileges: { corsEnabled: true, supportFetchAPI: true } }, { scheme: 'no-cors', privileges: { supportFetchAPI: true } }, + { scheme: 'no-cors-standard', privileges: { standard: true, supportFetchAPI: true } }, { scheme: 'no-fetch', privileges: { corsEnabled: true } }, { scheme: 'stream', privileges: { standard: true, stream: true } }, { scheme: 'foo', privileges: { standard: true } },