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 <sattard@anthropic.com>

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) &&

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

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