Permalink
Browse files

Add explicit "Rosetta Flash JSONP abuse" protection

  • Loading branch information...
dougwilson committed Jul 11, 2014
1 parent 5d03d0e commit f684a64df71a08c0caf2371aa305411df690a10f
Showing with 46 additions and 36 deletions.
  1. +2 −0 History.md
  2. +11 −4 lib/response.js
  3. +33 −32 test/res.jsonp.js
View
@@ -1,6 +1,8 @@
3.x
===
+ * add explicit "Rosetta Flash JSONP abuse" protection
+ - previous versions are not vulnerable; this is just explicit protection
* deprecate `res.redirect(url, status)` -- use `res.redirect(status, url)` instead
* fix `res.send(status, num)` to send `num` as json (not error)
* deps: basic-auth@1.0.0
View
@@ -266,19 +266,26 @@ res.jsonp = function(obj){
var callback = this.req.query[app.get('jsonp callback name')];
// content-type
- this.charset = this.charset || 'utf-8';
- this.get('Content-Type') || this.set('Content-Type', 'application/json');
+ if (!this.get('Content-Type')) {
+ this.charset = 'utf-8';
+ this.set('X-Content-Type-Options', 'nosniff');
+ this.set('Content-Type', 'application/json');
+ }
// fixup callback
if (Array.isArray(callback)) {
callback = callback[0];
}
// jsonp
- if (callback && 'string' === typeof callback) {
+ if (typeof callback === 'string' && callback.length !== 0) {
+ this.charset = 'utf-8';
+ this.set('X-Content-Type-Options', 'nosniff');
this.set('Content-Type', 'text/javascript');
var cb = callback.replace(/[^\[\]\w$.]/g, '');
- body = 'typeof ' + cb + ' === \'function\' && ' + cb + '(' + body + ');';
+
+ // the /**/ is a specific security mitigation for "Rosetta Flash JSONP abuse"
+ body = '/**/ typeof ' + cb + ' === \'function\' && ' + cb + '(' + body + ');';

This comment has been minimized.

Show comment
Hide comment
@stephenmathieson

stephenmathieson Jul 23, 2014

typeof was enough; the comment isn't necessary

@stephenmathieson

stephenmathieson Jul 23, 2014

typeof was enough; the comment isn't necessary

This comment has been minimized.

Show comment
Hide comment
@dougwilson

dougwilson Jul 23, 2014

Member

I am aware, but unfortunately without it, pen testing software will flag it as vulnerable.

@dougwilson

dougwilson Jul 23, 2014

Member

I am aware, but unfortunately without it, pen testing software will flag it as vulnerable.

This comment has been minimized.

Show comment
Hide comment
@dougwilson

dougwilson Jul 23, 2014

Member

It also doesn't help convincing the security vendors when all the major players are using /**/ as the solution. For example: https://api.github.com/?callback=foo

@dougwilson

dougwilson Jul 23, 2014

Member

It also doesn't help convincing the security vendors when all the major players are using /**/ as the solution. For example: https://api.github.com/?callback=foo

This comment has been minimized.

Show comment
Hide comment
}
return this.send(body);
View
@@ -14,11 +14,8 @@ describe('res', function(){
request(app)
.get('/?callback=something')
- .end(function(err, res){
- res.headers.should.have.property('content-type', 'text/javascript; charset=utf-8');
- res.text.should.equal('typeof something === \'function\' && something({"count":1});');
- done();
- })
+ .expect('Content-Type', 'text/javascript; charset=utf-8')
+ .expect(200, /something\(\{"count":1\}\);/, done);
})
it('should use first callback parameter with jsonp', function(done){
@@ -29,12 +26,9 @@ describe('res', function(){
});
request(app)
- .get('/?callback=something&callback=somethingelse')
- .end(function(err, res){
- res.headers.should.have.property('content-type', 'text/javascript; charset=utf-8');
- res.text.should.equal('typeof something === \'function\' && something({"count":1});');
- done();
- })
+ .get('/?callback=something&callback=somethingelse')
+ .expect('Content-Type', 'text/javascript; charset=utf-8')
+ .expect(200, /something\(\{"count":1\}\);/, done);
})
it('should ignore object callback parameter with jsonp', function(done){
@@ -61,11 +55,8 @@ describe('res', function(){
request(app)
.get('/?clb=something')
- .end(function(err, res){
- res.headers.should.have.property('content-type', 'text/javascript; charset=utf-8');
- res.text.should.equal('typeof something === \'function\' && something({"count":1});');
- done();
- })
+ .expect('Content-Type', 'text/javascript; charset=utf-8')
+ .expect(200, /something\(\{"count":1\}\);/, done);
})
it('should allow []', function(done){
@@ -77,11 +68,8 @@ describe('res', function(){
request(app)
.get('/?callback=callbacks[123]')
- .end(function(err, res){
- res.headers.should.have.property('content-type', 'text/javascript; charset=utf-8');
- res.text.should.equal('typeof callbacks[123] === \'function\' && callbacks[123]({"count":1});');
- done();
- })
+ .expect('Content-Type', 'text/javascript; charset=utf-8')
+ .expect(200, /callbacks\[123\]\(\{"count":1\}\);/, done);
})
it('should disallow arbitrary js', function(done){
@@ -93,11 +81,8 @@ describe('res', function(){
request(app)
.get('/?callback=foo;bar()')
- .end(function(err, res){
- res.headers.should.have.property('content-type', 'text/javascript; charset=utf-8');
- res.text.should.equal('typeof foobar === \'function\' && foobar({});');
- done();
- })
+ .expect('Content-Type', 'text/javascript; charset=utf-8')
+ .expect(200, /foobar\(\{\}\);/, done);
})
it('should escape utf whitespace', function(done){
@@ -109,13 +94,24 @@ describe('res', function(){
request(app)
.get('/?callback=foo')
- .end(function(err, res){
- res.headers.should.have.property('content-type', 'text/javascript; charset=utf-8');
- res.text.should.equal('typeof foo === \'function\' && foo({"str":"\\u2028 \\u2029 woot"});');
- done();
- });
+ .expect('Content-Type', 'text/javascript; charset=utf-8')
+ .expect(200, /foo\(\{"str":"\\u2028 \\u2029 woot"\}\);/, done);
});
+ it('should include security header and prologue', function (done) {
+ var app = express();
+
+ app.use(function(req, res){
+ res.jsonp({ count: 1 });
+ });
+
+ request(app)
+ .get('/?callback=something')
+ .expect('Content-Type', 'text/javascript; charset=utf-8')
+ .expect('X-Content-Type-Options', 'nosniff')
+ .expect(200, /^\/\*\*\//, done);
+ })
+
it('should not override previous Content-Types with no callback', function(done){
var app = express();
@@ -127,7 +123,11 @@ describe('res', function(){
request(app)
.get('/')
.expect('Content-Type', 'application/vnd.example+json; charset=utf-8')
- .expect(200, '{"hello":"world"}', done);
+ .expect(200, '{"hello":"world"}', function (err, res) {
+ if (err) return done(err);
+ res.headers.should.not.have.property('x-content-type-options');
+ done();
+ });
})
it('should override previous Content-Types with callback', function(done){
@@ -141,6 +141,7 @@ describe('res', function(){
request(app)
.get('/?callback=cb')
.expect('Content-Type', 'text/javascript; charset=utf-8')
+ .expect('X-Content-Type-Options', 'nosniff')
.expect(200, /cb\(\{"hello":"world"\}\);$/, done);
})

0 comments on commit f684a64

Please sign in to comment.