Express Open Redirect vulnerability
Research is free — Hunters explains how the bug works, the root-cause code pattern, how the fix addresses it, and how to test whether a target is affected, in chat. Investigate & write exploit is a paid run — the engine reads the advisory and fix commits, then builds and validates a working proof-of-concept exploit with reproduction steps.
Affected versions
3.4.5 → fixed in 4.0.0-rc1
Details
URL Redirection to Untrusted Site ('Open Redirect') vulnerability in Express. This vulnerability affects the use of the Express Response object. This issue impacts Express: from 3.4.5 before 4.0.0-rc1.
The fix
no semver2 so travis stops crying
lib/response.js+3 −7
@@ -605,8 +605,7 @@ res.cookie = function(name, val, options){/*** Set the location header to `url`.*-* The given `url` can also be the name of a mapped url, for-* example by default express supports "back" which redirects+* The given `url` can also be "back", which redirects* to the _Referrer_ or _Referer_ headers or "/".** Examples:@@ -637,11 +636,8 @@ res.location = function(url){, req = this.req, path;// setup redirect map-var map = { back: req.get('Referrer') || '/' };-// perform redirect-url = map[url] || url;+// "back" is an alias for the referrer+if(url === 'back') url = req.get('Referrer') || '/';// relativeif (!~url.indexOf('://') && 0 != url.indexOf('//')) {lib/response.js | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)
History.md+9 −0
@@ -1,3 +1,12 @@+3.4.5 / 2013-11-27+==================+* update connect+* res.location: remove leading ./ #1802 @kapouer+* res.redirect: fix `res.redirect('toString') #1829 @michaelficarra+* res.send: always send ETag when content-length > 0+* router: add Router.all() method+3.4.4 / 2013-10-29==================
lib/response.js+1 −1
@@ -132,7 +132,7 @@ res.send = function(body){// ETag support// TODO: W/ support-if (app.settings.etag && len > 1024 && 'GET' == req.method) {+if (app.settings.etag && len && 'GET' == req.method) {if (!this.get('ETag')) {this.set('ETag', etag(body));}
lib/response.js+2 −2
@@ -643,11 +643,11 @@ res.location = function(url){// relativeif (!~url.indexOf('://') && 0 != url.indexOf('//')) {-var path+var path;// relative to pathif ('.' == url[0]) {-path = req.originalUrl.split('?')[0]+path = req.originalUrl.split('?')[0];url = path + ('/' == path[path.length - 1] ? '' : '/') + url;// relative to mount-point} else if ('/' != url[0]) {pathslib/response.js | 9 +++++----1 file changed, 5 insertions(+), 4 deletions(-)
lib/response.js+5 −4
@@ -14,6 +14,7 @@ var http = require('http'), cookie = require('cookie'), send = require('send'), mime = connect.mime+, resolve = require('url').resolve, basename = path.basename, extname = path.extname;@@ -633,7 +634,8 @@ res.cookie = function(name, val, options){res.location = function(url){var app = this.app-, req = this.req;+, req = this.req+, path;// setup redirect mapvar map = { back: req.get('Referrer') || '/' };@@ -643,12 +645,11 @@ res.location = function(url){// relativeif (!~url.indexOf('://') && 0 != url.indexOf('//')) {-var path;-// relative to pathif ('.' == url[0]) {path = req.originalUrl.split('?')[0];-url = path + ('/' == path[path.length - 1] ? '' : '/') + url;+path = path + ('/' == path[path.length - 1] ? '' : '/');+url = resolve(path, url);// relative to mount-point} else if ('/' != url[0]) {path = app.path();test/res.location.js | 4 ++--test/res.redirect.js | 10 +++++-----2 files changed, 7 insertions(+), 7 deletions(-)
test/res.redirect.js+5 −5
@@ -210,17 +210,17 @@ describe('res', function(){request(root).get('/depth1').end(function(err, res){-res.headers.should.have.property('location', '/depth1/./index');+res.headers.should.have.property('location', '/depth1/index');request(root).get('/depth1/depth2').end(function(err, res){-res.headers.should.have.property('location', '/depth1/depth2/./index');+res.headers.should.have.property('location', '/depth1/depth2/index');request(root).get('/depth1/depth2/depth3').end(function(err, res){-res.headers.should.have.property('location', '/depth1/depth2/depth3/./index');+res.headers.should.have.property('location', '/depth1/depth2/depth3/index');done();})})@@ -231,11 +231,11 @@ describe('res', function(){request(root).get('/depth2').end(function(err, res){-res.headers.should.have.property('location', '/depth2/./index');+res.headers.should.have.property('location', '/depth2/index');request(root).get('/depth3').end(function(err, res){-res.headers.should.have.property('location', '/depth3/./index');+res.headers.should.have.property('location', '/depth3/index');done();})})test/res.location.js | 17 +++++++++++++++++1 file changed, 17 insertions(+)
lib/response.js+1 −1
@@ -637,7 +637,7 @@ res.location = function(url){, path;// "back" is an alias for the referrer-if(url === 'back') url = req.get('Referrer') || '/';+if ('back' == url) url = req.get('Referrer') || '/';// relativeif (!~url.indexOf('://') && 0 != url.indexOf('//')) {examples/content-negotiation/index.js | 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)
package.json+1 −1
@@ -43,7 +43,7 @@"should": "2","connect-redis": "*","marked": "*",-"supertest": "0.8.1 - 1"+"supertest": ">= 0.8.1 < 1"},"keywords": ["express",lib/response.js | 2 +-test/res.send.js | 15 +++++++++++++++2 files changed, 16 insertions(+), 1 deletion(-)
test/res.send.js+15 −0
@@ -321,6 +321,21 @@ describe('res', function(){describe('"etag" setting', function(){describe('when enabled', function(){+it('should send ETag even when content-length < 1024', function(done){+var app = express();+app.use(function(req, res){+res.send('kajdslfkasdf');+});+request(app)+.get('/')+.end(function(err, res){+res.headers.should.have.property('etag');+done();+});+})+it('should send ETag ', function(done){var app = express();test/res.sendfile.js | 2 +-1 file changed, 1 insertion(+), 1 deletion(-)
test/res.sendfile.js+1 −1
@@ -51,7 +51,7 @@ describe('res', function(){.get('/').end(function(err, res){assert(1 == calls, 'called too many times');-res.text.should.equal("ENOENT, stat 'test/fixtures/nope.html'");+res.text.should.startWith("ENOENT, stat");res.statusCode.should.equal(200);done();});.travis.yml | 2 ++package.json | 18 +++++++++---------2 files changed, 11 insertions(+), 9 deletions(-)
.travis.yml+2 −0
@@ -2,3 +2,5 @@ language: node_jsnode_js:- "0.8"- "0.10"+before_install:+- "npm update -g npm"
package.json+9 −9
@@ -32,18 +32,18 @@"methods": "0.1.0","send": "0.1.4","cookie-signature": "1.0.1",-"debug": "*"+"debug": "0.7.3 - 1"},"devDependencies": {-"ejs": "*",-"mocha": "*",+"ejs": "0.8.4 - 1",+"mocha": "^1.13.0","jade": "0.30.0",-"hjs": "*",-"stylus": "*",-"should": "2",-"connect-redis": "*",-"marked": "*",-"supertest": ">= 0.8.1 < 1"+"hjs": "0.0.6 - 1",+"stylus": "0.39.1 - 1",+"should": "^2.0.2",+"connect-redis": "^1.4.5",+"marked": "0.2.9 - 1",+"supertest": "0.8.1 - 1"},"keywords": ["express",.travis.yml | 4 +---package.json | 20 ++++++++++----------2 files changed, 11 insertions(+), 13 deletions(-)
.travis.yml+1 −3
@@ -1,6 +1,4 @@language: node_jsnode_js:- "0.8"-- "0.10"-before_install:-- "npm update -g npm"+- "0.10"
package.json+10 −10
@@ -32,18 +32,18 @@"methods": "0.1.0","send": "0.1.4","cookie-signature": "1.0.1",-"debug": "0.7.3 - 1"+"debug": ">= 0.7.3 < 1"},"devDependencies": {-"ejs": "0.8.4 - 1",-"mocha": "^1.13.0",+"ejs": ">= 0.8.4 < 1",+"mocha": ">= 1.13.0 < 2","jade": "0.30.0",-"hjs": "0.0.6 - 1",-"stylus": "0.39.1 - 1",-"should": "^2.0.2",-"connect-redis": "^1.4.5",-"marked": "0.2.9 - 1",-"supertest": "0.8.1 - 1"+"hjs": ">= 0.0.6 < 1",+"stylus": ">= 0.39.1 < 1",+"should": ">= 2.0.2 < 3",+"connect-redis": ">= 1.4.5 < 2",+"marked": ">= 0.2.9 < 1",+"supertest": ">= 0.8.1 < 1"},"keywords": ["express",@@ -66,6 +66,6 @@"test": "make test"},"engines": {-"node": "*"+"node": ">= 0.8.0"}}lib/response.js | 4 ++--1 file changed, 2 insertions(+), 2 deletions(-)
test/res.location.js+2 −2
@@ -64,7 +64,7 @@ describe('res', function(){request(app).get('/post/1').end(function(err, res){-res.headers.should.have.property('location', '/post/1/./edit');+res.headers.should.have.property('location', '/post/1/edit');done();})})@@ -81,7 +81,7 @@ describe('res', function(){request(app).get('/post/1').end(function(err, res){-res.headers.should.have.property('location', '/post/1/../new');+res.headers.should.have.property('location', '/post/new');done();})})
More files changed — see the full commit.