origins not checked when url has sid
Nobody has claimed this yet.
- Dominant language
- TypeScript
- Stars
- 63.2k
- Forks
- 10.3k
- Avg merge
- 11d 20h
- Merged PRs (30d)
- 2
Description
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.
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in lib/server.js at Server.prototype.verify, focusing on the sid branch and the allowRequest call. Reproduce the described requests with an allowed origin and a sid from an untrusted origin, then verify that all requests are checked against allowed origins rather than only the initial handshake.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- javascript, node.js
- Domain
- backend, security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100