Movatterモバイル変換

[0]ホーム
URL:

Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

 Sign up
Appearance settings

Commit0867302

Browse files
FDrag0njonchurch
authored andcommitted
Prevent open redirect allow list bypass due to encodeurl
Co-authored-by: Jon Church <me@jonchurch.com>
1 parent567c9c6 commit0867302

File tree

2 files changed

+64
-2
lines changed

2 files changed

+64
-2
lines changed

‎lib/response.js‎

Lines changed: 19 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ var extname = path.extname;
3434
varmime=send.mime;
3535
varresolve=path.resolve;
3636
varvary=require('vary');
37+
varurlParse=require('url').parse;
3738

3839
/**
3940
* Response prototype.
@@ -911,8 +912,25 @@ res.location = function location(url) {
911912
loc=this.req.get('Referrer')||'/';
912913
}
913914

915+
varlowerLoc=loc.toLowerCase();
916+
varencodedUrl=encodeUrl(loc);
917+
if(lowerLoc.indexOf('https://')===0||lowerLoc.indexOf('http://')===0){
918+
try{
919+
varparsedUrl=urlParse(loc);
920+
varparsedEncodedUrl=urlParse(encodedUrl);
921+
// Because this can encode the host, check that we did not change the host
922+
if(parsedUrl.host!==parsedEncodedUrl.host){
923+
// If the host changes after encodeUrl, return the original url
924+
returnthis.set('Location',loc);
925+
}
926+
}catch(e){
927+
// If parse fails, return the original url
928+
returnthis.set('Location',loc);
929+
}
930+
}
931+
914932
// set location
915-
returnthis.set('Location',encodeUrl(loc));
933+
returnthis.set('Location',encodedUrl);
916934
};
917935

918936
/**

‎test/res.location.js‎

Lines changed: 45 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,27 @@
11
'use strict'
22

33
varexpress=require('../')
4-
,request=require('supertest');
4+
,request=require('supertest')
5+
,url=require('url');
56

67
describe('res',function(){
78
describe('.location(url)',function(){
89
it('should set the header',function(done){
910
varapp=express();
1011

12+
app.use(function(req,res){
13+
res.location('http://google.com/').end();
14+
});
15+
16+
request(app)
17+
.get('/')
18+
.expect('Location','http://google.com/')
19+
.expect(200,done)
20+
})
21+
22+
it('should preserve trailing slashes when not present',function(done){
23+
varapp=express();
24+
1125
app.use(function(req,res){
1226
res.location('http://google.com').end();
1327
});
@@ -31,6 +45,36 @@ describe('res', function(){
3145
.expect(200,done)
3246
})
3347

48+
it('should not encode bad "url"',function(done){
49+
varapp=express()
50+
51+
app.use(function(req,res){
52+
// This is here to show a basic check one might do which
53+
// would pass but then the location header would still be bad
54+
if(url.parse(req.query.q).host!=='google.com'){
55+
res.status(400).end('Bad url');
56+
}
57+
res.location(req.query.q).end();
58+
});
59+
60+
request(app)
61+
.get('/?q=http://google.com\\@apple.com')
62+
.expect(200)
63+
.expect('Location','http://google.com\\@apple.com')
64+
.end(function(err){
65+
if(err){
66+
throwerr;
67+
}
68+
69+
// This ensures that our protocol check is case insensitive
70+
request(app)
71+
.get('/?q=HTTP://google.com\\@apple.com')
72+
.expect(200)
73+
.expect('Location','HTTP://google.com\\@apple.com')
74+
.end(done)
75+
});
76+
});
77+
3478
it('should not touch already-encoded sequences in "url"',function(done){
3579
varapp=express()
3680

0 commit comments

Comments
 (0)
[8]ページ先頭
©2009-2025 Movatter.jp