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

Please clarify how the location could leak in example 3 (about shortlinks) #87

Open
jub0bs opened this issue Oct 27, 2022 · 10 comments
Open

Comments

@jub0bs
Copy link
Contributor

jub0bs commented Oct 27, 2022

Example 3 reads like this:

MegaCorp’s leak-prevention department is worried, though, that this access will allow external folks to read the location of any redirect that the shortener would return. [...] MegaCorp’s shortlink engineers are careful to avoid this potential failure by returning CORS headers only for the preflight. The "real" navigation doesn’t require CORS headers, and they don’t actually want to support cross-origin requests as being CORS-same-origin

I'm not sure I understand how the location could leak, even if the ACAO header was present on the response to the actual (preflighted) request... Since Location is not a CORS-safelisted response-header name, client code would only be able to read the value of that header if the response listed the header name in question in its Access-Control-Expose-Headers header.

Am I missing something?

@annevk
Copy link

annevk commented Oct 28, 2022

Well, the design of Access-Control-Expose-Headers did not take Spectre and friends into account. Implementations could maybe do better there, but some headers that are not safe listed need nevertheless be processed, so that might be tricky to completely pull off. (This should probably be tracked in Fetch at some point, though I'm not aware of anyone actively being interested in fixing this.)

cc @arturjanc

@mikewest
Copy link
Member

I think Chromium is handling this correctly today. It was certainly not handling this correctly when I wrote it a while back (we piped redirects back to the renderer to handle frame-src and form-action). I don't know the state of other browsers these days; hopefull we've generally aligned on handling this kind of thing in a privileged process.

Regardless, the example could probably be reformulated to make a different point: handle CORS for the preflight, but don't enable CORS access for the resource itself. That would likely enable origins to use fetch() to read data they shouldn't, unless you're very careful about evaluating fetch metadata headers and etc. Would you like to take a stab at that @jub0bs ?

@annevk
Copy link

annevk commented Oct 28, 2022

@mikewest does Chrome actually remove the Location header for opaque-redirect filtered responses before passing those to the website process? Or for a normal CORS response that happens to have a Location header in it that's not safelisted? (I wasn't referring to the normal case of following redirects, which should indeed happen in the network process, but maybe this document is.)

@mikewest
Copy link
Member

@annevk: You mean something like fetch('...', { redirect: 'manual' })? I don't think we do. (And I also see more subresource redirect-handling code in our renderer process than I thought we had. So we might not be handling this as high up the stack as I thought after all.)

@annevk
Copy link

annevk commented Oct 28, 2022

Yeah, and also just fetch('https://cross-origin.example/') in general as you might think that secrets in response headers that are not safelisted using Access-Control-Expose-Headers are safe, but you would be mistaken. (But arguably you shouldn't be mistaken and browsers should make more of an effort.)

@arturjanc
Copy link

FWIW in our original internal Spectre-related remediation we assumed that a response to a known URL with a Location header redirecting to a user-dependent location (e.g. something with a secret per-user token) could leak, and tried to protect some endpoints with this behavior via CORP and server-side logic using Fetch Metadata headers. However, we haven't been very consistent about this and were hoping that work on OOR-CORS and related changes in browsers would make this less of a concern in the long run.

@jub0bs
Copy link
Contributor Author

jub0bs commented Nov 8, 2022

Thank you all for your input.

@mikewest

Would you like to take a stab at that @jub0bs ?

Sorry for the late reply. Now that I have a better understanding of the issue, I can certainly take a stab at it.

@letitz
Copy link
Collaborator

letitz commented Mar 2, 2023

Hey @jub0bs, are you still interested in sending a PR for this?

@jub0bs
Copy link
Contributor Author

jub0bs commented Mar 2, 2023

@letitz Yes! Sorry, I forgot about this. I'll see what I can do this weekend.

@letitz
Copy link
Collaborator

letitz commented Mar 2, 2023

No worries, and thanks for contributing :)

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

5 participants