Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Release 4.19.0 #5551

Merged
merged 2 commits into from
Mar 20, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion History.md
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
unreleased
4.18.3 / 2024-03-20
==========

* Prevent open redirect allow list bypass due to encodeurl
* deps: cookie@0.6.0

4.18.3 / 2024-02-29
Expand Down
20 changes: 19 additions & 1 deletion lib/response.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ var extname = path.extname;
var mime = send.mime;
var resolve = path.resolve;
var vary = require('vary');
var urlParse = require('url').parse;

/**
* Response prototype.
Expand Down Expand Up @@ -911,8 +912,25 @@ res.location = function location(url) {
loc = this.req.get('Referrer') || '/';
}

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

// set location
return this.set('Location', encodeUrl(loc));
return this.set('Location', encodedUrl);
};

/**
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
{
"name": "express",
"description": "Fast, unopinionated, minimalist web framework",
"version": "4.18.3",
"version": "4.19.0",
"author": "TJ Holowaychuk <tj@vision-media.ca>",
"contributors": [
"Aaron Heckmann <aaron.heckmann+github@gmail.com>",
Expand Down
46 changes: 45 additions & 1 deletion test/res.location.js
Original file line number Diff line number Diff line change
@@ -1,13 +1,27 @@
'use strict'

var express = require('../')
, request = require('supertest');
, request = require('supertest')
, url = require('url');

describe('res', function(){
describe('.location(url)', function(){
it('should set the header', function(done){
var app = express();

app.use(function(req, res){
res.location('http://google.com/').end();
});

request(app)
.get('/')
.expect('Location', 'http://google.com/')
.expect(200, done)
})

it('should preserve trailing slashes when not present', function(done){
var app = express();

app.use(function(req, res){
res.location('http://google.com').end();
});
Expand All @@ -31,6 +45,36 @@ describe('res', function(){
.expect(200, done)
})

it('should not encode bad "url"', function (done) {
var app = express()

app.use(function (req, res) {
// This is here to show a basic check one might do which
// would pass but then the location header would still be bad
if (url.parse(req.query.q).host !== 'google.com') {
res.status(400).end('Bad url');
}
res.location(req.query.q).end();
});

request(app)
.get('/?q=http://google.com\\@apple.com')
.expect(200)
.expect('Location', 'http://google.com\\@apple.com')
.end(function (err) {
if (err) {
throw err;
}

// This ensures that our protocol check is case insensitive
request(app)
.get('/?q=HTTP://google.com\\@apple.com')
.expect(200)
.expect('Location', 'HTTP://google.com\\@apple.com')
.end(done)
});
});

it('should not touch already-encoded sequences in "url"', function (done) {
var app = express()

Expand Down
Loading