fix: ensure corsEnabled: false protocol handlers do not work across protocols (42-x-y) (#51269)

fix: ensure corsEnabled: false protocol handlers do not work across protocols (#51152)

* fix: ensure corsEnabled: false protocol handlers do not work across protocols

Subresource requests for registered custom protocols are routed to
ElectronURLLoaderFactory via the renderer's per-scheme URLLoaderFactoryBundle
entry, which bypasses the network service's CorsURLLoaderFactory. This meant a
cross-origin page could fetch() a scheme registered with {supportFetchAPI: true}
and read the response body even when {corsEnabled: true} was not set.

Replicate CorsURLLoader::StartRequest's kCorsDisabledScheme gate in
ElectronURLLoaderFactory::CreateLoaderAndStart so cross-origin mode=cors
requests to such schemes fail before the JS handler runs, and tag cross-origin
mode=no-cors responses as opaque so the body is not script-readable while <img>
and similar subresource loads continue to work.

Re-enable the long-disabled "disallows CORS and fetch requests when only
supportFetchAPI is specified" test, add coverage for the opaque/no-cors,
same-origin, handler-not-invoked, corsEnabled-unaffected and net.fetch-unaffected
cases, and migrate spec helpers that were exercising a {supportFetchAPI: true}
scheme cross-origin to a corsEnabled scheme.

* chore: oxfmt

(cherry picked from commit 92f0993d94)

shell/browser/net/electron_url_loader_factory.cc

@@ -4,6 +4,7 @@
 
 #include "shell/browser/net/electron_url_loader_factory.h"
 
+#include <algorithm>
 #include <memory>
 #include <string>
 #include <string_view>
@@ -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<network::mojom::URLLoaderClient> 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. <img>, 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

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('<!doctype html><body>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('<!doctype html><body>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 <img> 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<Request>(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) {

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');
       }
     });
 

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",

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 } },