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

Timing-Allow-Origin check should be case-insensitive #202

Closed
npm1 opened this issue Feb 11, 2019 · 6 comments
Closed

Timing-Allow-Origin check should be case-insensitive #202

npm1 opened this issue Feb 11, 2019 · 6 comments

Comments

@npm1
Copy link
Contributor

npm1 commented Feb 11, 2019

Scheme, host, port are all case-insensitive, so the timing allow check algorithm should do a case-insensitive check. Currently, the check is stated as If the Timing-Allow-Origin header value list does not contain a value which is byte-for-byte identical to the serialization of the current document's origin, nor a wildcard ("*"), return fail.

@yoavweiss
Copy link
Contributor

I agree current language doesn't handle case sensitivity in a tolerant way. Looking at CORS it doesn't seem to do that either.

IIRC, @annevk was the one pointing me towards the "byte-for-byte identical" language, so he may have opinions here. Also, I'm not sure what current implementations do. Looking at Chromium's, it doesn't seem case insensitive. (other than its "null" comparison...)

We should at the very least add tests to see that whatever we decide here is actually enforced.

@annevk
Copy link
Member

annevk commented Feb 12, 2019

Hmm, ideally the null comparison is case-sensitive too.

@npm1 what are you basing your assertions on?

@yoavweiss
Copy link
Contributor

Hmm, ideally the null comparison is case-sensitive too.

Agreed

@yoavweiss
Copy link
Contributor

From IRC discussion with @annevk, it seems like the origin with which the headers are compared will always be lower case. That means developers should always set the TAO (and ACAO) header values to lower case and the checks will pass fine, even if the links the user followed or the URL they clicked contained uppercase letter as part of the scheme or the host.

So, I don't see any reason to change the current behavior to be case-insensitive.

@npm1
Copy link
Contributor Author

npm1 commented Feb 12, 2019

This was reported based on a question from a security developer. Two strings that are case-insensitive matches correspond to the same origin but I guess at some point the URL parser lowercases these so there is a 'canonical' way to represent them? In particular, 'Serializing a request origin' probably already returns this in the canonical way. It seems fine to keep this case-sensitive for consistency then.

@npm1 npm1 closed this as completed Feb 12, 2019
@annevk
Copy link
Member

annevk commented Feb 13, 2019

Indeed, if the browser ever exposed them in non-canonical case that would be problematic. And if we start doing non-literal comparisons developers might expect we do other kinds of parsing behavior too, which we never do for origins.

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

3 participants