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

Integration with document.open() #402

Closed
TimothyGu opened this issue Jun 30, 2019 · 7 comments
Closed

Integration with document.open() #402

TimothyGu opened this issue Jun 30, 2019 · 7 comments

Comments

@TimothyGu
Copy link
Member

Currently, CSP does not integrate with document.open() at all, though user agents already do so. The crux here is that while CSP uses responses as the main object for information of page load, document.open() does not touch the network at all and hence does not have a response to pass into CSP algorithms.. @andypaicu listed out some things that are necessary for such an integration in whatwg/html#4510 (comment):

  1. Add another initialize-type hook (initialize-document-csp-from-document, name WIP) that takes a target (document) and a source (document)
  2. Do something like steps 2 and 3 of initialize-document-csp to copy the policies and initialize directives
  3. Remove the response paramater from the initialize directive algorithm, or make it optional. I am unaware of any directive that actually uses this.

… except all of these are TODOs for CSP rather than for HTML. In particular, the proposed initialize-document-csp-from-document hook already exists in implementations in some form.

@annevk
Copy link
Member

annevk commented Jan 25, 2023

I stumbled across https://bugs.chromium.org/p/chromium/issues/detail?id=836148 which has some motivation for this behavior. But I don't fully understand it. It seems to me the attacker could just as well append their script element to 1.txt. Does it really make sense to have complicated policy container inheritance when document.open() is used given that?

cc @antosart

@antosart
Copy link
Member

I agree. When designing the policy container inheritance, we though about the document.open() case. My feeling is that document.open() is somehow very similar to removing all nodes from the dom with document.removeChild() and then appending new nodes. So keeping the policies made sense to me.

I also do not see the security issue with that.

CC @mikewest, since I remember discussions on the general question: whether non-html resources in an iframe src (like <iframe src='img.jpg'>) should inherit the parent's CSP, to protect against this sort of problems.

@mikewest
Copy link
Member

That approach makes sense to me, @antosart.

For <iframe src="img">, I think Chrome's current behavior is to accept headers on the image and apply them to the document we synthesize. I think that's what HTML would have told us to do as well, though I'm having a hard time finding it as I'm still catching up on the recent navigation refactoring. We could do something more draconian if this is a problem in practice (e.g. all images loaded as documents are sandboxed or etc), but I'm not sure it is.

@annevk
Copy link
Member

annevk commented Jan 27, 2023

Right, that follows from https://html.spec.whatwg.org/#navigate-media (well, its caller).

I suspect that websites will indeed forget to set headers on such responses at times, but I'm not convinced we want to make that our problem as inheritance is hardly simple.

@annevk
Copy link
Member

annevk commented Aug 17, 2023

Okay, so what remains here is some tests that document.open() does not in fact exhibit this behavior and then have the user agents that do have special behavior there, remove it. (OP could be read to state that all user agents do this, but that's not the case, and as argued above it doesn't help you anyway.)

@antosart
Copy link
Member

antosart commented Sep 5, 2023

@annevk
Copy link
Member

annevk commented Sep 5, 2023

Thanks, those look good.

@annevk annevk closed this as not planned Won't fix, can't repro, duplicate, stale Sep 5, 2023
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

4 participants