origins not checked when url has sid

[barisusakli]

You want to:

• report a bug
• request a feature

Current behaviour

When the url does not have the sid parameter the request is checked against the allowed origins here, after this subsequent requests are not checked against allowed origins.

Steps to reproduce

Set origins to mydomain.com
Make a request from evildomain.com spoof the origin header to match mydomain.com and get a valid sid.
Make request from evildomain with the valid sid and origin is no longer checked.

Expected behaviour

All requests to be checked against allowed origins

Other information (e.g. stacktraces, related issues, suggestions how to fix)

I changed the code to this and seems to work but wanted to get some feedback.

Server.prototype.verify = function (req, upgrade, fn) {
  // transport check
  var transport = req._query.transport;
  if (!~this.transports.indexOf(transport)) {
    debug('unknown transport "%s"', transport);
    return fn(Server.errors.UNKNOWN_TRANSPORT, false);
  }

  // 'Origin' header check
  var isOriginInvalid = checkInvalidHeaderChar(req.headers.origin);
  if (isOriginInvalid) {
    req.headers.origin = null;
    return fn(Server.errors.BAD_REQUEST, false);
  }

  // sid check
  var sid = req._query.sid;
  if (sid) {
    if (!this.clients.hasOwnProperty(sid)) {
      return fn(Server.errors.UNKNOWN_SID, false);
    }
    if (!upgrade && this.clients[sid].transport.name !== transport) {
      debug('bad request: unexpected transport without upgrade');
      return fn(Server.errors.BAD_REQUEST, false);
    }
  } else {
    // handshake is GET only
    if ('GET' !== req.method) return fn(Server.errors.BAD_HANDSHAKE_METHOD, false);
  }
  if (!this.allowRequest) return fn(null, true);
  return this.allowRequest(req, fn);
};

This calls allowRequest function for all requests instead of just the initial one.

[kevinhodges]

Seeing similar symptoms, be good to get a resolution!