Skip to content

Commit

Permalink
Improved fix for open redirect allow list bypass
Browse files Browse the repository at this point in the history
Co-authored-by: Jon Church <me@jonchurch.com>
Co-authored-by: Blake Embrey <hello@blakeembrey.com>
  • Loading branch information
3 people committed Mar 25, 2024
1 parent 4f0f6cc commit 0b74695
Show file tree
Hide file tree
Showing 3 changed files with 280 additions and 63 deletions.
5 changes: 5 additions & 0 deletions History.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,8 @@
unreleased
==========

* Improved fix for open redirect allow list bypass

4.19.1 / 2024-03-20
==========

Expand Down
31 changes: 11 additions & 20 deletions lib/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand All @@ -56,6 +55,7 @@ module.exports = res
*/

var charsetRegExp = /;\s*charset\s*=/;
var schemaAndHostRegExp = /^(?:[a-zA-Z][a-zA-Z0-9+.-]*:)?\/\/[^\\\/\?]+/;

/**
* Set status `code`.
Expand Down Expand Up @@ -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('http://') === 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);
};

/**
Expand Down
Loading

0 comments on commit 0b74695

Please sign in to comment.