cors icon indicating copy to clipboard operation
cors copied to clipboard

Unexpected response when requesting with multiple comma separated origins and wildcard AllowedOrigins

Open latonz opened this issue 3 years ago • 6 comments
trafficstars

If an origin https://*.example.com is allowed and a client sends a request with a header Origin: https://github.com,https://test.example.com the wildcard match is performed against the comma separated origin header value and succeeds. This leads to an unexpected and according to the specification invalid response header Access-Control-Allow-Origin: https://github.com,https://test.example.com. Also it was never intended that https://github.com should be allowed.

As far as I know current Browser implementations never send comma separated values in origin headers (and never multiple headers) and I'm not even sure if the specification would allow this. However, I think it would make sense to validate the origin header's format and reject requests with an unexpected format (not sure on how to do this backward compatible?).

latonz avatar Aug 03 '22 09:08 latonz

We can validate origin header yes. I don't see how this will pose backward compatible issue if we cover all formats browsers may send.

rs avatar Aug 03 '22 09:08 rs

I think the hard part is to cover all formats a browser may send. I don't think the change itself is breaking, but the risk of introducing a breaking change for some edge cases not correctly covered by the validation should be considered. Therefore it may be a good option to add a flag to disable the validation.

Would you accept a PR? Do you have any expectations on how this validation should be implemented?

latonz avatar Aug 03 '22 12:08 latonz

Sure for PR. Expectation no dep, no regex, doesn't break :)

rs avatar Aug 03 '22 12:08 rs

@rs Out of curiosity, why do you insist on not relying on regexps? I do agree with you, but I'd like to understand your reasons.

jub0bs avatar Oct 29 '23 09:10 jub0bs

It's slow and unnecessary

rs avatar Oct 29 '23 10:10 rs

@latonz

I'm not even sure if the specification would allow this.

I'm a bit late to the party but,

  1. because "Origin" is a so-called forbidden request-header name, no client running in a Fetch-compliant browser can include such a header in its requests; only the browser itself can;
  2. Fetch-compliant browsers either omit to include an Origin header or include one that contains exactly one (no less, no more) origin value.

Therefore, because no request issued from a Fetch-compliant browser can include the following header,

Origin: https://github.com,https://test.example.com

allowing https://*.example.com in your configuration shouldn't be cause for concern (at least if your server indeed trusts all subdomains of example.com).

However, I do believe that the range of origin patterns supported by rs/cors is too broad and easy to abuse; for instance, rs/cors supports https://* as an origin pattern, but it arguably shouldn't, because such a pattern is dangerously overpermissive.

I think it would make sense to validate the origin header's format and reject requests with an unexpected format.

FWIW, some CORS libraries do parse or validate the value of the Origin header. Mine parses it because it needs to determine its constituents (scheme, host, port) to figure out whether the origin is allowed; the performance penalty for doing so is minimal. rs/cors too could parse/validate the Origin header.

jub0bs avatar Apr 18 '24 08:04 jub0bs