Improved fix for open redirect allow list bypass

wesleytodd · jonchurch · blakeembrey · wesleytodd · commit 0b746953c4bd · 2024-03-25T09:29:41.000-05:00
Co-authored-by: Jon Church &lt;me@jonchurch.com&gt;
Co-authored-by: Blake Embrey &lt;hello@blakeembrey.com&gt;
diff --git a/History.md b/History.md
@@ -1,3 +1,8 @@
+unreleased
+==========
+
+  * Improved fix for open redirect allow list bypass
+
 4.19.1 / 2024-03-20
 ==========
 
diff --git a/lib/response.js b/lib/response.js
@@ -34,7 +34,6 @@ var extname = path.extname;
 var mime = send.mime;
 var resolve = path.resolve;
 var vary = require('vary');
-var urlParse = require('url').parse;
 
 /**
  * Response prototype.
@@ -56,6 +55,7 @@ module.exports = res
  */
 
 var charsetRegExp = /;\s*charset\s*=/;
+var schemaAndHostRegExp = /^(?:[a-zA-Z][a-zA-Z0-9+.-]*:)?\/\/[^\\\/\?]+/;
 
 /**
  * Set status `code`.
@@ -905,32 +905,23 @@ res.cookie = function (name, value, options) {
  */
 
 res.location = function location(url) {
-  var loc = String(url);
+  var loc;
 
   // "back" is an alias for the referrer
   if (url === 'back') {
     loc = this.req.get('Referrer') || '/';
+  } else {
+    loc = String(url);
   }
 
-  var lowerLoc = loc.toLowerCase();
-  var encodedUrl = encodeUrl(loc);
-  if (lowerLoc.indexOf('https://') === 0 || lowerLoc.indexOf('https://') === 0) {
-    try {
-      var parsedUrl = urlParse(loc);
-      var parsedEncodedUrl = urlParse(encodedUrl);
-      // Because this can encode the host, check that we did not change the host
-      if (parsedUrl.host !== parsedEncodedUrl.host) {
-        // If the host changes after encodeUrl, return the original url
-        return this.set('Location', loc);
-      }
-    } catch (e) {
-      // If parse fails, return the original url
-      return this.set('Location', loc);
-    }
-  }
+  var m = schemaAndHostRegExp.exec(loc);
+  var pos = m ? m[0].length + 1 : 0;
+
+  // Only encode after host to avoid invalid encoding which can introduce
+  // vulnerabilities (e.g. `\\` to `%5C`).
+  loc = loc.slice(0, pos) + encodeUrl(loc.slice(pos));
 
-  // set location
-  return this.set('Location', encodedUrl);
+  return this.set('Location', loc);
 };
 
 /**
diff --git a/test/res.location.js b/test/res.location.js
