search

agrueneberg / Corser

CORS middleware for Node.js
MIT License
91 stars 11 forks source link

Returns Access-Control-Allow-Credentials Header for All Origins #21

Open dbohannon opened 7 years ago

dbohannon commented 7 years ago

When corser is applied without the origins option, the ACAO header is set to *. However, if it is applied without the origins option AND the supportsCredentials option is enabled, the middleware silently reflects the requesting origin in the ACAO header. This leaves the application open to cross-domain attacks since any origin can read the response to credentialed requests.

The relevant portion of code is located at https://github.com/agrueneberg/Corser/blob/master/lib/corser.js#L163-L169

Allowing arbitrary origins to read credentialed responses is specifically forbidden in the CORS spec. I suggest warning the developer when the supportsCredentials option is enabled with an undefined origins option, or leaving the ACAO header as * and letting the cors-compliant browsers reject the cross-origin response due to improper CORS headers.

agrueneberg commented 7 years ago

Hi David, thanks for your detailed report! I'm currently busy with moving to a different state, but I will get back to you as soon as I can.

agrueneberg commented 7 years ago

I just had a quick look at the spec and you are right: I must have missed the big fat green *_Note: The string "" cannot be used for a resource that supports credentials_**. The spec doesn't specify what to do in those cases, so your suggestions to fix the problem may not be enough to be compliant. Unfortunately, the initialization is synchronous, so we have to pick one of the ugly solutions of throwing or returning an error if origins: [] is paired with supportsCredentials: true, unless there are some new developments I am not aware of (I haven't done any JS in almost two years).

dbohannon commented 7 years ago

If you don't wish to throw an error then I would suggest leaving the ACAO header as the wildcard * rather than reflecting the origin. A compliant browser will refuse to share the response cross-origin due to the invalid CORS configuration, which is more secure than reflecting the origin.

agrueneberg commented 7 years ago

Sorry for getting back to you so late. I don't like either throwing an error or relying on compliance on the browser side (a lesson I've learned in #18). What do you think of generating a warning and flipping supportCredentials to false if origins: [] is paired with supportCredentials: true? I'd hate to be the guy who has to figure out what's wrong if logging is not properly set up, but as far as I know it's not a commonly used feature anyway, so this may be the path of least harm...