Skip to content

Commit a169050

Browse files
revert: fix(security): do not allow all origins by default
This reverts commit f78a575. This commit contains a breaking change which deviates from semver, which we try to follow as closely as possible. That's why this change is reverted and we will rather suggest users to upgrade to v3. Related: #3741
1 parent 873fdc5 commit a169050

File tree

2 files changed

+47
-23
lines changed

2 files changed

+47
-23
lines changed

lib/index.js

+23-10
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,7 @@ function Server(srv, opts){
5454
this.parser = opts.parser || parser;
5555
this.encoder = new this.parser.Encoder();
5656
this.adapter(opts.adapter || Adapter);
57-
this.origins(opts.origins || []);
57+
this.origins(opts.origins || '*:*');
5858
this.sockets = this.of('/');
5959
if (srv) this.attach(srv, opts);
6060
}
@@ -67,18 +67,31 @@ function Server(srv, opts){
6767
*/
6868

6969
Server.prototype.checkRequest = function(req, fn) {
70-
const origin = req.headers.origin;
70+
var origin = req.headers.origin || req.headers.referer;
7171

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

75+
if (!!origin && typeof(this._origins) == 'function') return this._origins(origin, fn);
76+
if (this._origins.indexOf('*:*') !== -1) return fn(null, true);
7677
if (origin) {
77-
fn(null, this._origins.includes(origin));
78-
} else {
79-
const noOriginIsValid = this._origins.length === 0;
80-
fn(null, noOriginIsValid);
78+
try {
79+
var parts = url.parse(origin);
80+
var defaultPort = 'https:' == parts.protocol ? 443 : 80;
81+
parts.port = parts.port != null
82+
? parts.port
83+
: defaultPort;
84+
var ok =
85+
~this._origins.indexOf(parts.protocol + '//' + parts.hostname + ':' + parts.port) ||
86+
~this._origins.indexOf(parts.hostname + ':' + parts.port) ||
87+
~this._origins.indexOf(parts.hostname + ':*') ||
88+
~this._origins.indexOf('*:' + parts.port);
89+
debug('origin %s is %svalid', origin, !!ok ? '' : 'not ');
90+
return fn(null, !!ok);
91+
} catch (ex) {
92+
}
8193
}
94+
fn(null, false);
8295
};
8396

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

227-
this._origins = typeof v === 'string' ? [v] : v;
240+
this._origins = v;
228241
return this;
229242
};
230243

test/socket.io.js

+24-13
Original file line numberDiff line numberDiff line change
@@ -73,7 +73,7 @@ describe('socket.io', function(){
7373
it('should be able to set origins to engine.io', function() {
7474
var srv = io(http());
7575
srv.set('origins', 'http://hostname.com:*');
76-
expect(srv.origins()).to.eql(['http://hostname.com:*']);
76+
expect(srv.origins()).to.be('http://hostname.com:*');
7777
});
7878

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

265+
it('should allow request when origin defined an the same is specified', function(done) {
266+
var sockets = io({ origins: 'http://foo.example:*' }).listen('54015');
267+
request.get('http://localhost:54015/socket.io/default/')
268+
.set('origin', 'http://foo.example')
269+
.query({ transport: 'polling' })
270+
.end(function (err, res) {
271+
expect(res.status).to.be(200);
272+
done();
273+
});
274+
});
275+
265276
it('should allow request when origin defined as function and same is supplied', function(done) {
266277
var sockets = io({ origins: function(origin,callback){
267278
if (origin == 'http://foo.example') {
@@ -296,7 +307,7 @@ describe('socket.io', function(){
296307

297308
it('should allow request when origin defined as function and no origin is supplied', function(done) {
298309
var sockets = io({ origins: function(origin,callback){
299-
if (origin === undefined) {
310+
if (origin == '*') {
300311
return callback(null, true);
301312
}
302313
return callback(null, false);
@@ -309,6 +320,17 @@ describe('socket.io', function(){
309320
});
310321
});
311322

323+
it('should default to port 443 when protocol is https', function(done) {
324+
var sockets = io({ origins: 'https://foo.example:443' }).listen('54036');
325+
request.get('http://localhost:54036/socket.io/default/')
326+
.set('origin', 'https://foo.example')
327+
.query({ transport: 'polling' })
328+
.end(function (err, res) {
329+
expect(res.status).to.be(200);
330+
done();
331+
});
332+
});
333+
312334
it('should allow request if custom function in opts.allowRequest returns true', function(done){
313335
var sockets = io(http().listen(54022), { allowRequest: function (req, callback) {
314336
return callback(null, true);
@@ -345,17 +367,6 @@ describe('socket.io', function(){
345367
done();
346368
});
347369
});
348-
349-
it('should disallow any origin by default', (done) => {
350-
io().listen('54025');
351-
request.get('http://localhost:54025/socket.io/default/')
352-
.set('origin', 'https://foo.example')
353-
.query({ transport: 'polling' })
354-
.end((err, res) => {
355-
expect(res.status).to.be(403);
356-
done();
357-
});
358-
});
359370
});
360371

361372
describe('close', function(){

0 commit comments

Comments
 (0)