From 9c1e9e02371cbe1898aa199bfe1c783d3162efcc Mon Sep 17 00:00:00 2001 From: "trop[bot]" <37223003+trop[bot]@users.noreply.github.com> Date: Sat, 11 Apr 2026 10:46:11 -0500 Subject: [PATCH] fix: apply IsSafeRedirectTarget to net module redirects (#50930) Co-authored-by: trop[bot] <37223003+trop[bot]@users.noreply.github.com> Co-authored-by: Sam Attard --- shell/common/api/electron_api_url_loader.cc | 11 ++++ spec/api-net-custom-protocols-spec.ts | 25 +++++++++ spec/api-net-spec.ts | 62 +++++++++++++++++++++ 3 files changed, 98 insertions(+) diff --git a/shell/common/api/electron_api_url_loader.cc b/shell/common/api/electron_api_url_loader.cc index a5f591989c..1f4711db1c 100644 --- a/shell/common/api/electron_api_url_loader.cc +++ b/shell/common/api/electron_api_url_loader.cc @@ -18,11 +18,13 @@ #include "base/notreached.h" #include "base/sequence_checker.h" #include "content/public/browser/global_request_id.h" +#include "content/public/common/url_utils.h" #include "gin/object_template_builder.h" #include "mojo/public/cpp/bindings/remote.h" #include "mojo/public/cpp/system/data_pipe_producer.h" #include "net/base/auth.h" #include "net/base/load_flags.h" +#include "net/base/net_errors.h" #include "net/http/http_util.h" #include "net/url_request/redirect_util.h" #include "services/network/public/cpp/resource_request.h" @@ -776,6 +778,15 @@ void SimpleURLLoaderWrapper::OnRedirect( // The redirect was aborted by JS. return; + if (!content::IsSafeRedirectTarget(url_before_redirect, + redirect_info.new_url)) { + auto self = weak_factory_.GetWeakPtr(); + Emit("error", net::ErrorToString(net::ERR_UNSAFE_REDIRECT)); + if (self) + Cancel(); + return; + } + // Optimization: if both the old and new URLs are handled by the network // service, just FollowRedirect. if (network::IsURLHandledByNetworkService(redirect_info.new_url) && diff --git a/spec/api-net-custom-protocols-spec.ts b/spec/api-net-custom-protocols-spec.ts index 374bea8114..d712c9b8d4 100644 --- a/spec/api-net-custom-protocols-spec.ts +++ b/spec/api-net-custom-protocols-spec.ts @@ -55,6 +55,31 @@ describe('net module custom protocols', () => { expect(body).to.equal('hello electron-test://bar'); }); + it('can be redirected from http to a custom scheme', async () => { + protocol.interceptStringProtocol('http', (req, cb) => { cb({ statusCode: 302, headers: { location: 'electron-test://bar' } }); }); + defer(() => { + protocol.uninterceptProtocol('http'); + }); + protocol.registerStringProtocol('electron-test', (req, cb) => { cb('hello ' + req.url); }); + defer(() => { + protocol.unregisterProtocol('electron-test'); + }); + const body = await net.fetch('http://foo').then(r => r.text()); + expect(body).to.equal('hello electron-test://bar'); + }); + + it('can be redirected from file to file', async () => { + protocol.interceptStringProtocol('file', (req, cb) => { + if (/\/redirect-me$/.test(req.url)) return cb({ statusCode: 302, headers: { location: 'file:///target' } }); + cb('redirected-to ' + req.url); + }); + defer(() => { + protocol.uninterceptProtocol('file'); + }); + const body = await net.fetch('file:///redirect-me').then(r => r.text()); + expect(body).to.equal('redirected-to file:///target'); + }); + it('should not follow redirect when redirect: error', async () => { protocol.registerStringProtocol('electron-test', (req, cb) => { if (/redirect/.test(req.url)) return cb({ statusCode: 302, headers: { location: 'electron-test://bar' } }); diff --git a/spec/api-net-spec.ts b/spec/api-net-spec.ts index 8d486250e2..44faa75449 100644 --- a/spec/api-net-spec.ts +++ b/spec/api-net-spec.ts @@ -927,6 +927,18 @@ describe('net module', () => { await once(urlRequest, 'error'); }); + test('should emit an error when redirected from http to file:', async () => { + const serverUrl = await respondOnce.toSingleURL((request, response) => { + response.statusCode = 302; + response.setHeader('Location', 'file:///'); + response.end(); + }); + const urlRequest = net.request({ url: serverUrl }); + urlRequest.end(); + const [err] = await once(urlRequest, 'error'); + expect(String(err)).to.match(/ERR_UNSAFE_REDIRECT/); + }); + test('should follow redirect when handler calls callback', async () => { const serverUrl = await respondOnce.toRoutes({ '/redirectChain': (request, response) => { @@ -1598,6 +1610,56 @@ describe('net module', () => { await expect(r).to.be.rejectedWith(/ERR_NAME_NOT_RESOLVED/); }); + test('should follow a redirect to another http origin', async () => { + const targetUrl = await respondOnce.toSingleURL((req, res) => { + res.end('redirected'); + }); + const serverUrl = await respondOnce.toSingleURL((req, res) => { + res.statusCode = 302; + res.setHeader('Location', targetUrl); + res.end(); + }); + const resp = await net.fetch(serverUrl); + expect(resp.status).to.equal(200); + expect(await resp.text()).to.equal('redirected'); + }); + + test('should reject a redirect from http to file:', async () => { + const serverUrl = await respondOnce.toSingleURL((req, res) => { + res.statusCode = 302; + res.setHeader('Location', 'file:///'); + res.end(); + }); + await expect(net.fetch(serverUrl)).to.be.rejectedWith(/ERR_UNSAFE_REDIRECT/); + }); + + test('should reject a redirect from http to data:', async () => { + const serverUrl = await respondOnce.toSingleURL((req, res) => { + res.statusCode = 302; + res.setHeader('Location', 'data:text/plain,hello'); + res.end(); + }); + await expect(net.fetch(serverUrl)).to.be.rejectedWith(/ERR_UNSAFE_REDIRECT/); + }); + + test('should reject a redirect from http to about:', async () => { + const serverUrl = await respondOnce.toSingleURL((req, res) => { + res.statusCode = 302; + res.setHeader('Location', 'about:blank'); + res.end(); + }); + await expect(net.fetch(serverUrl)).to.be.rejectedWith(/ERR_UNSAFE_REDIRECT/); + }); + + test('should reject a redirect from http to blob:', async () => { + const serverUrl = await respondOnce.toSingleURL((req, res) => { + res.statusCode = 302; + res.setHeader('Location', 'blob:https://example.com/00000000-0000-0000-0000-000000000000'); + res.end(); + }); + await expect(net.fetch(serverUrl)).to.be.rejectedWith(/ERR_UNSAFE_REDIRECT/); + }); + test('should reject body promise when stream fails', async () => { const serverUrl = await respondOnce.toSingleURL((request, response) => { response.write('first chunk');