From b6a41dd270e5b67f8014b41c0c35a97365ae227d Mon Sep 17 00:00:00 2001
From: Joshua Rogers <MegaManSec@users.noreply.github.com>
Date: Wed, 6 Aug 2025 11:57:46 +0200
Subject: [PATCH] fix(router): securely check path for proper destination

The router previously checked whether a path contained a route key. This check
was insufficient, as it allowed a route configured as such:
    '/admin': 'http://localhost:4002',
to be accessed to be accessible by requesting `/some/public/path?q=/admin`,
which would match and route the request to localhost:4002.

If http-proxy-middleware is in front of a firewall which restricts access to
/admin, this could have allowed unauthorized access to the admin route.
---
 src/router.ts            | 42 +++++++++++++++++++++++++---------------
 test/unit/router.spec.ts | 14 ++++++++++++++
 2 files changed, 40 insertions(+), 16 deletions(-)

diff --git a/src/router.ts b/src/router.ts
index d5f72975..2c66ba48 100644
--- a/src/router.ts
+++ b/src/router.ts
@@ -18,31 +18,41 @@ export async function getTarget(req, config) {
 }
 
 function getTargetFromProxyTable(req, table) {
-  let result;
   const host = req.headers.host;
   const path = req.url;
-
-  const hostAndPath = host + path;
+  let hostMatch;
 
   for (const [key, value] of Object.entries(table)) {
-    if (containsPath(key)) {
-      if (hostAndPath.indexOf(key) > -1) {
-        // match 'localhost:3000/api'
-        result = value;
-        debug('match: "%s" -> "%s"', key, result);
-        break;
-      }
-    } else {
+    // host-only rule
+    if (!containsPath(key)) {
       if (key === host) {
-        // match 'localhost:3000'
-        result = value;
-        debug('match: "%s" -> "%s"', host, result);
-        break;
+        hostMatch = value;
+      }
+      continue;
+    }
+
+    // If key starts with '/', it's a path-only rule.
+    if (key.startsWith('/')) {
+      if (path.startsWith(key)) {
+        debug('path-only match: "%s" -> "%s"', key, value);
+        return value;
+      }
+    }
+    // If key contains a '/' but doesn't start with one, it's a host+path rule.
+    else {
+      const hostAndPath = host + path;
+      if (hostAndPath.startsWith(key)) {
+        debug('host+path match: "%s" -> "%s"', key, value);
+        return value;
       }
     }
   }
 
-  return result;
+  // If we finished the loop with no path-involved matches, use the host-only match.
+  if (hostMatch) {
+    debug('host-only fallback: "%s" -> "%s"', host, hostMatch);
+  }
+  return hostMatch;
 }
 
 function containsPath(v) {
diff --git a/test/unit/router.spec.ts b/test/unit/router.spec.ts
index 1b1f0fc6..72bb5038 100644
--- a/test/unit/router.spec.ts
+++ b/test/unit/router.spec.ts
@@ -157,5 +157,19 @@ describe('router unit test', () => {
         return expect(result).resolves.toBe('http://localhost:6006');
       });
     });
+
+    describe('exact path tests, rather than partial match', () => {
+      it('should NOT match route key from query string parameters', () => {
+        fakeReq.url = '/foobar?a=1&b=/rest';
+        result = getTarget(fakeReq, proxyOptionWithRouter);
+        return expect(result).resolves.toBeUndefined();
+      });
+
+      it('should match from the start of the path and not a substring', () => {
+        fakeReq.url = '/some/path/that/also/contains/rest';
+        result = getTarget(fakeReq, proxyOptionWithRouter);
+        return expect(result).resolves.toBe('http://localhost:6007');
+      });
+    });
   });
 });