diff --git a/Contributing.md b/Contributing.md index dfe5f13833..a9ba84690c 100644 --- a/Contributing.md +++ b/Contributing.md @@ -118,11 +118,13 @@ nominate someone to take their place. TC members will be added as admin's on the Github orgs, npm orgs, and other resources as necessary to be effective in the role. -To remain "active" a TC member should have participation within the last 6 months and miss -no more than three consecutive TC meetings. Members who do not meet this are expected to step down. -If A TC member does not step down, an issue can be opened in the discussions repo to move them -to inactive status. TC members who step down or are removed due to inactivity will be moved -into inactive status. +To remain "active" a TC member should have participation within the last 12 months and miss +no more than six consecutive TC meetings. Our goal is to increase participation, not punish +people for any lack of participation, this guideline should be only be used as such +(replace an inactive member with a new active one, for example). Members who do not meet this +are expected to step down. If A TC member does not step down, an issue can be opened in the +discussions repo to move them to inactive status. TC members who step down or are removed due +to inactivity will be moved into inactive status. Inactive status members can become active members by self nomination if the TC is not already larger than the maximum of 10. They will also be given preference if, while at max size, an @@ -154,7 +156,21 @@ dissent. When the PR is merged, a TC member will add them to the proper GitHub/ ### Current Project Captains -- `expressjs.com`: @crandmck -- `multer`: @LinusU -- `path-to-regexp`: @blakeembrey -- `router`: @dougwilson +- `expressjs/express`: @wesleytodd +- `expressjs/discussions`: @wesleytodd +- `expressjs/expressjs.com`: @crandmck +- `expressjs/body-parser`: @wesleytodd +- `expressjs/multer`: @LinusU +- `expressjs/cookie-parser`: @wesleytodd +- `expressjs/generator`: @wesleytodd +- `expressjs/statusboard`: @wesleytodd +- `pillarjs/path-to-regexp`: @blakeembrey +- `pillarjs/router`: @dougwilson, @wesleytodd +- `pillarjs/finalhandler`: @wesleytodd +- `pillarjs/request`: @wesleytodd +- `jshttp/http-errors`: @wesleytodd +- `jshttp/cookie`: @wesleytodd +- `jshttp/on-finished`: @wesleytodd +- `jshttp/forwarded`: @wesleytodd +- `jshttp/proxy-addr`: @wesleytodd + diff --git a/History.md b/History.md index 0784045557..ac2e7cf719 100644 --- a/History.md +++ b/History.md @@ -1,4 +1,20 @@ -4.18.3 / 2024-02-26 +4.19.2 / 2024-03-25 +========== + + * Improved fix for open redirect allow list bypass + +4.19.1 / 2024-03-20 +========== + + * Allow passing non-strings to res.location with new encoding handling checks + +4.19.0 / 2024-03-20 +========== + + * Prevent open redirect allow list bypass due to encodeurl + * deps: cookie@0.6.0 + +4.18.3 / 2024-02-29 ========== * Fix routing requests without method @@ -6,6 +22,8 @@ - Fix strict json error message on Node.js 19+ - deps: content-type@~1.0.5 - deps: raw-body@2.5.2 + * deps: cookie@0.6.0 + - Add `partitioned` option 4.18.2 / 2022-10-08 =================== diff --git a/Release-Process.md b/Release-Process.md index ae740972f7..55e6218925 100644 --- a/Release-Process.md +++ b/Release-Process.md @@ -184,3 +184,9 @@ $ npm publish **NOTE:** The version number to publish will be picked up automatically from package.json. + +### Step 7. Update documentation website + +The documentation website https://expressjs.com/ documents the current release version in various places. For a new release: +1. Change the value of `current_version` in https://github.com/expressjs/expressjs.com/blob/gh-pages/_data/express.yml to match the latest version number. +2. Add a new section to the change log. For example, for a 4.x release, https://github.com/expressjs/expressjs.com/blob/gh-pages/en/changelog/4x.md, diff --git a/lib/response.js b/lib/response.js index fede486c06..dd7b3c8201 100644 --- a/lib/response.js +++ b/lib/response.js @@ -55,6 +55,7 @@ module.exports = res */ var charsetRegExp = /;\s*charset\s*=/; +var schemaAndHostRegExp = /^(?:[a-zA-Z][a-zA-Z0-9+.-]*:)?\/\/[^\\\/\?]+/; /** * Set status `code`. @@ -904,15 +905,23 @@ res.cookie = function (name, value, options) { */ res.location = function location(url) { - var loc = url; + var loc; // "back" is an alias for the referrer if (url === 'back') { loc = this.req.get('Referrer') || '/'; + } else { + loc = String(url); } - // set location - return this.set('Location', encodeUrl(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)); + + return this.set('Location', loc); }; /** diff --git a/package.json b/package.json index c3845d2d4b..f299d882b0 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "name": "express", "description": "Fast, unopinionated, minimalist web framework", - "version": "4.18.3", + "version": "4.19.2", "author": "TJ Holowaychuk ", "contributors": [ "Aaron Heckmann ", @@ -33,7 +33,7 @@ "body-parser": "1.20.2", "content-disposition": "0.5.4", "content-type": "~1.0.4", - "cookie": "0.5.0", + "cookie": "0.6.0", "cookie-signature": "1.0.6", "debug": "2.6.9", "depd": "2.0.0", diff --git a/test/res.cookie.js b/test/res.cookie.js index 93deb76988..c837820605 100644 --- a/test/res.cookie.js +++ b/test/res.cookie.js @@ -82,6 +82,22 @@ describe('res', function(){ }) }) + describe('partitioned', function () { + it('should set partitioned', function (done) { + var app = express(); + + app.use(function (req, res) { + res.cookie('name', 'tobi', { partitioned: true }); + res.end(); + }); + + request(app) + .get('/') + .expect('Set-Cookie', 'name=tobi; Path=/; Partitioned') + .expect(200, done) + }) + }) + describe('maxAge', function(){ it('should set relative expires', function(done){ var app = express(); diff --git a/test/res.location.js b/test/res.location.js index 158afac01e..c80b38de6b 100644 --- a/test/res.location.js +++ b/test/res.location.js @@ -1,7 +1,9 @@ 'use strict' var express = require('../') - , request = require('supertest'); + , request = require('supertest') + , assert = require('assert') + , url = require('url'); describe('res', function(){ describe('.location(url)', function(){ @@ -9,38 +11,38 @@ describe('res', function(){ var app = express(); app.use(function(req, res){ - res.location('http://google.com').end(); + res.location('http://google.com/').end(); }); request(app) .get('/') - .expect('Location', 'http://google.com') + .expect('Location', 'http://google.com/') .expect(200, done) }) - it('should encode "url"', function (done) { - var app = express() + it('should preserve trailing slashes when not present', function(done){ + var app = express(); - app.use(function (req, res) { - res.location('https://google.com?q=\u2603 §10').end() - }) + app.use(function(req, res){ + res.location('http://google.com').end(); + }); request(app) .get('/') - .expect('Location', 'https://google.com?q=%E2%98%83%20%C2%A710') + .expect('Location', 'http://google.com') .expect(200, done) }) - it('should not touch already-encoded sequences in "url"', function (done) { + it('should encode "url"', function (done) { var app = express() app.use(function (req, res) { - res.location('https://google.com?q=%A710').end() + res.location('https://google.com?q=\u2603 §10').end() }) request(app) .get('/') - .expect('Location', 'https://google.com?q=%A710') + .expect('Location', 'https://google.com?q=%E2%98%83%20%C2%A710') .expect(200, done) }) @@ -101,5 +103,283 @@ describe('res', function(){ .expect(200, done) }) }) + + it('should encode data uri', function (done) { + var app = express() + app.use(function (req, res) { + res.location('data:text/javascript,export default () => { }').end(); + }); + + request(app) + .get('/') + .expect('Location', 'data:text/javascript,export%20default%20()%20=%3E%20%7B%20%7D') + .expect(200, done) + }) + + it('should encode data uri', function (done) { + var app = express() + app.use(function (req, res) { + res.location('data:text/javascript,export default () => { }').end(); + }); + + request(app) + .get('/') + .expect('Location', 'data:text/javascript,export%20default%20()%20=%3E%20%7B%20%7D') + .expect(200, done) + }) + + it('should consistently handle non-string input: boolean', function (done) { + var app = express() + app.use(function (req, res) { + res.location(true).end(); + }); + + request(app) + .get('/') + .expect('Location', 'true') + .expect(200, done) + }); + + it('should consistently handle non-string inputs: object', function (done) { + var app = express() + app.use(function (req, res) { + res.location({}).end(); + }); + + request(app) + .get('/') + .expect('Location', '[object%20Object]') + .expect(200, done) + }); + + it('should consistently handle non-string inputs: array', function (done) { + var app = express() + app.use(function (req, res) { + res.location([]).end(); + }); + + request(app) + .get('/') + .expect('Location', '') + .expect(200, done) + }); + + it('should consistently handle empty string input', function (done) { + var app = express() + app.use(function (req, res) { + res.location('').end(); + }); + + request(app) + .get('/') + .expect('Location', '') + .expect(200, done) + }); + + + if (typeof URL !== 'undefined') { + it('should accept an instance of URL', function (done) { + var app = express(); + + app.use(function(req, res){ + res.location(new URL('http://google.com/')).end(); + }); + + request(app) + .get('/') + .expect('Location', 'http://google.com/') + .expect(200, done); + }); + } }) + + describe('location header encoding', function() { + function createRedirectServerForDomain (domain) { + var app = express(); + app.use(function (req, res) { + var host = url.parse(req.query.q, false, true).host; + // This is here to show a basic check one might do which + // would pass but then the location header would still be bad + if (host !== domain) { + res.status(400).end('Bad host: ' + host + ' !== ' + domain); + } + res.location(req.query.q).end(); + }); + return app; + } + + function testRequestedRedirect (app, inputUrl, expected, expectedHost, done) { + return request(app) + // Encode uri because old supertest does not and is required + // to test older node versions. New supertest doesn't re-encode + // so this works in both. + .get('/?q=' + encodeURIComponent(inputUrl)) + .expect('') // No body. + .expect(200) + .expect('Location', expected) + .end(function (err, res) { + if (err) { + console.log('headers:', res.headers) + console.error('error', res.error, err); + return done(err, res); + } + + // Parse the hosts from the input URL and the Location header + var inputHost = url.parse(inputUrl, false, true).host; + var locationHost = url.parse(res.headers['location'], false, true).host; + + assert.strictEqual(locationHost, expectedHost); + + // Assert that the hosts are the same + if (inputHost !== locationHost) { + return done(new Error('Hosts do not match: ' + inputHost + " !== " + locationHost)); + } + + return done(null, res); + }); + } + + it('should not touch already-encoded sequences in "url"', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + 'https://google.com?q=%A710', + 'https://google.com?q=%A710', + 'google.com', + done + ); + }); + + it('should consistently handle relative urls', function (done) { + var app = createRedirectServerForDomain(null); + testRequestedRedirect( + app, + '/foo/bar', + '/foo/bar', + null, + done + ); + }); + + it('should not encode urls in such a way that they can bypass redirect allow lists', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + 'http://google.com\\@apple.com', + 'http://google.com\\@apple.com', + 'google.com', + done + ); + }); + + it('should not be case sensitive', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + 'HTTP://google.com\\@apple.com', + 'HTTP://google.com\\@apple.com', + 'google.com', + done + ); + }); + + it('should work with https', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + 'https://google.com\\@apple.com', + 'https://google.com\\@apple.com', + 'google.com', + done + ); + }); + + it('should correctly encode schemaless paths', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + '//google.com\\@apple.com/', + '//google.com\\@apple.com/', + 'google.com', + done + ); + }); + + it('should percent encode backslashes in the path', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + 'https://google.com/foo\\bar\\baz', + 'https://google.com/foo%5Cbar%5Cbaz', + 'google.com', + done + ); + }); + + it('should encode backslashes in the path after the first backslash that triggered path parsing', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + 'https://google.com\\@app\\l\\e.com', + 'https://google.com\\@app%5Cl%5Ce.com', + 'google.com', + done + ); + }); + + it('should escape header splitting for old node versions', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + 'http://google.com\\@apple.com/%0d%0afoo:%20bar', + 'http://google.com\\@apple.com/%0d%0afoo:%20bar', + 'google.com', + done + ); + }); + + it('should encode unicode correctly', function (done) { + var app = createRedirectServerForDomain(null); + testRequestedRedirect( + app, + '/%e2%98%83', + '/%e2%98%83', + null, + done + ); + }); + + it('should encode unicode correctly even with a bad host', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + 'http://google.com\\@apple.com/%e2%98%83', + 'http://google.com\\@apple.com/%e2%98%83', + 'google.com', + done + ); + }); + + it('should work correctly despite using deprecated url.parse', function (done) { + var app = createRedirectServerForDomain('google.com'); + testRequestedRedirect( + app, + 'https://google.com\'.bb.com/1.html', + 'https://google.com\'.bb.com/1.html', + 'google.com', + done + ); + }); + + it('should encode file uri path', function (done) { + var app = createRedirectServerForDomain(''); + testRequestedRedirect( + app, + 'file:///etc\\passwd', + 'file:///etc%5Cpasswd', + '', + done + ); + }); + }); })