Skip to content

Commit

Permalink
fix(security): do not allow all origins by default
Browse files Browse the repository at this point in the history
BREAKING CHANGE: previously, all origins were allowed by default, which
meant that a Socket.IO server sent the necessary CORS headers
(`Access-Control-Allow-xxx`) to any domain by default.

Please note that you are not impacted if:

- you are using Socket.IO v2 and the `origins` option to restrict the list of allowed domains
- you are using Socket.IO v3 (disabled by default)

This commit also removes the support for '*' matchers and protocol-less
URL:

```
io.origins('https://example.com:443'); => io.origins(['https://example.com']);
io.origins('localhost:3000');          => io.origins(['http://localhost:3000']);
io.origins('http://localhost:*');      => io.origins(['http://localhost:3000']);
io.origins('*:3000');                  => io.origins(['http://localhost:3000']);
```

To restore the previous behavior (please use with caution):

```js
io.origins((_, callback) => {
  callback(null, true);
});
```

See also:

- https://developer.mozilla.org/en-US/docs/Web/HTTP/CORS
- https://socket.io/docs/v3/handling-cors/
- https://socket.io/docs/v3/migrating-from-2-x-to-3-0/#CORS-handling

Thanks a lot to https://github.com/ni8walk3r for the security report.
  • Loading branch information
darrachequesne committed Jan 4, 2021
1 parent d33a619 commit f78a575
Show file tree
Hide file tree
Showing 2 changed files with 23 additions and 47 deletions.
33 changes: 10 additions & 23 deletions lib/index.js
Expand Up @@ -54,7 +54,7 @@ function Server(srv, opts){
this.parser = opts.parser || parser;
this.encoder = new this.parser.Encoder();
this.adapter(opts.adapter || Adapter);
this.origins(opts.origins || '*:*');
this.origins(opts.origins || []);
this.sockets = this.of('/');
if (srv) this.attach(srv, opts);
}
Expand All @@ -67,31 +67,18 @@ function Server(srv, opts){
*/

Server.prototype.checkRequest = function(req, fn) {
var origin = req.headers.origin || req.headers.referer;
const origin = req.headers.origin;

// file:// URLs produce a null Origin which can't be authorized via echo-back
if ('null' == origin || null == origin) origin = '*';
if (typeof this._origins === 'function') {
return this._origins(origin, fn);
}

if (!!origin && typeof(this._origins) == 'function') return this._origins(origin, fn);
if (this._origins.indexOf('*:*') !== -1) return fn(null, true);
if (origin) {
try {
var parts = url.parse(origin);
var defaultPort = 'https:' == parts.protocol ? 443 : 80;
parts.port = parts.port != null
? parts.port
: defaultPort;
var ok =
~this._origins.indexOf(parts.protocol + '//' + parts.hostname + ':' + parts.port) ||
~this._origins.indexOf(parts.hostname + ':' + parts.port) ||
~this._origins.indexOf(parts.hostname + ':*') ||
~this._origins.indexOf('*:' + parts.port);
debug('origin %s is %svalid', origin, !!ok ? '' : 'not ');
return fn(null, !!ok);
} catch (ex) {
}
fn(null, this._origins.includes(origin));
} else {
const noOriginIsValid = this._origins.length === 0;
fn(null, noOriginIsValid);
}
fn(null, false);
};

/**
Expand Down Expand Up @@ -237,7 +224,7 @@ Server.prototype.adapter = function(v){
Server.prototype.origins = function(v){
if (!arguments.length) return this._origins;

this._origins = v;
this._origins = typeof v === 'string' ? [v] : v;
return this;
};

Expand Down
37 changes: 13 additions & 24 deletions test/socket.io.js
Expand Up @@ -73,7 +73,7 @@ describe('socket.io', function(){
it('should be able to set origins to engine.io', function() {
var srv = io(http());
srv.set('origins', 'http://hostname.com:*');
expect(srv.origins()).to.be('http://hostname.com:*');
expect(srv.origins()).to.eql(['http://hostname.com:*']);
});

it('should be able to set authorization and send error packet', function(done) {
Expand Down Expand Up @@ -262,17 +262,6 @@ describe('socket.io', function(){
});
});

it('should allow request when origin defined an the same is specified', function(done) {
var sockets = io({ origins: 'http://foo.example:*' }).listen('54015');
request.get('http://localhost:54015/socket.io/default/')
.set('origin', 'http://foo.example')
.query({ transport: 'polling' })
.end(function (err, res) {
expect(res.status).to.be(200);
done();
});
});

it('should allow request when origin defined as function and same is supplied', function(done) {
var sockets = io({ origins: function(origin,callback){
if (origin == 'http://foo.example') {
Expand Down Expand Up @@ -307,7 +296,7 @@ describe('socket.io', function(){

it('should allow request when origin defined as function and no origin is supplied', function(done) {
var sockets = io({ origins: function(origin,callback){
if (origin == '*') {
if (origin === undefined) {
return callback(null, true);
}
return callback(null, false);
Expand All @@ -320,17 +309,6 @@ describe('socket.io', function(){
});
});

it('should default to port 443 when protocol is https', function(done) {
var sockets = io({ origins: 'https://foo.example:443' }).listen('54036');
request.get('http://localhost:54036/socket.io/default/')
.set('origin', 'https://foo.example')
.query({ transport: 'polling' })
.end(function (err, res) {
expect(res.status).to.be(200);
done();
});
});

it('should allow request if custom function in opts.allowRequest returns true', function(done){
var sockets = io(http().listen(54022), { allowRequest: function (req, callback) {
return callback(null, true);
Expand Down Expand Up @@ -367,6 +345,17 @@ describe('socket.io', function(){
done();
});
});

it('should disallow any origin by default', (done) => {
io().listen('54025');
request.get('http://localhost:54025/socket.io/default/')
.set('origin', 'https://foo.example')
.query({ transport: 'polling' })
.end((err, res) => {
expect(res.status).to.be(403);
done();
});
});
});

describe('close', function(){
Expand Down

0 comments on commit f78a575

Please sign in to comment.