Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
dbohannon opened this issue Mar 21, 2017 · 4 comments
Open

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

dbohannon opened this issue Mar 21, 2017 · 4 comments

Comments

@dbohannon
Copy link

dbohannon commented Mar 21, 2017

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
Copy link
Owner

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
Copy link
Owner

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
Copy link
Author

dbohannon commented Mar 23, 2017

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
Copy link
Owner

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...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants