Open Bug 1528790 Opened 5 years ago Updated 2 years ago

Resources inserted by parent frame into child frame respecting parent frame CSP rather than child frame CSP

Categories

(Core :: DOM: Security, defect, P3)

65 Branch
defect

Tracking

()

People

(Reporter: me, Unassigned)

References

Details

(Whiteboard: [domsecurity-backlog1])

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_14_1) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/72.0.3626.109 Safari/537.36

Steps to reproduce:

  1. Save the following HTML files:

index.html:

<meta http-equiv="Content-Security-Policy" content="script-src 'unsafe-inline'">


<script>                                                                    
const srcdoc = `

<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js">                                                                         
<\/script>

`;
const src = `data:text/html;charset=utf-8,${encodeURIComponent(srcdoc)}`;

window.onload = () => {
  const relayIframe = document.createElement('iframe');
  relayIframe.src = 'relay.html';
  relayIframe.onload = () => {
    const iframe = document.createElement('iframe');
    iframe.src = src;
    relayIframe.contentDocument.body.appendChild(iframe);
  };
  document.body.appendChild(relayIframe);
};

</script>

relay.html:

<meta http-equiv="Content-Security-Policy" content="script-src https:">
  1. Start a localhost server that serves these two HTML files and visit index.html.

Actual results:

Firefox blocks loading the JQuery script resource with the following error:

Content Security Policy: The page’s settings blocked the loading of a resource at https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js (“script-src”).

Expected results:

The jQuery script resource loads successfully.

This is the current behavior on Chrome, Safari and Opera. The JQuery script tag, although inserted by the top frame (index.html) via direct scripting of subframes, is inside a data: URL iframe inside another "relay" iframe, and the relay iframe has a CSP that allows any HTTPS resources for scripts. However, Firefox uses the CSP of top frame, which only allows inline scripts, to decide whether to ban the jQuery script tag.

Removing the top frame CSP makes the script tag loads successfully. I've also tried further restricting the CSP of the relay frame to have script-src 'none', but the jQuery script tag is still allowed, so it looks like Firefox is only looking at the CSP of the top frame in this case.

Haven't found the relevant section of the spec on this, so not sure which browsers' behaviors are correct.

Looks like the code snippet isn't escaped properly... Trying again with Markdown:

index.html:

<meta http-equiv="Content-Security-Policy" content="script-src 'unsafe-inline'">


<script>                                                                    
const srcdoc = `

<script src="https://ajax.googleapis.com/ajax/libs/jquery/3.3.1/jquery.min.js">                                                                         
<\/script>

`;
const src = `data:text/html;charset=utf-8,${encodeURIComponent(srcdoc)}`;

window.onload = () => {
  const relayIframe = document.createElement('iframe');
  relayIframe.src = 'relay.html';
  relayIframe.onload = () => {
    const iframe = document.createElement('iframe');
    iframe.src = src;
    relayIframe.contentDocument.body.appendChild(iframe);
  };
  document.body.appendChild(relayIframe);
};

</script>

relay.html:

<meta http-equiv="Content-Security-Policy" content="script-src https:">
Component: Untriaged → DOM: Security
Product: Firefox → Core

Tricky. What does Chrome do? I think we're arguably correct: Given frames PARENT -> RELAY -> INNER

PARENT is the one that created INNER, and at that point we inherit the creating-script's CSP. Then it's inserted into RELAY, but that's no longer relevant.

If this were not script-inserted but instead the relay.html contained the iframe in its markup then clearly the data: url frame would inherit from RELAY so maybe we're doing it wrong? the spec is not all that clear (I'd expect this to be in the HTML spec, not the the CSP spec).

Anne: any thoughts on which is correct?

Flags: needinfo?(annevk)

This is probably a consequence of us storing the CSP for attributes when those attributes get set, along with the principal. We did that for extensions, but if it's web-visible then we may want to rethink the exact details of how we do it. See also bug 1529877 comment 2.

For example, maybe we should only use the CSP of the stored principal if it's an extension principal or something.

https://w3c.github.io/webappsec-csp/#initialize-document-csp uses request's client. Given that the <iframe> src attribute is set (and not something like location of its contentWindow) the "source browsing context" (this concept is somewhat broken, but okay) ends up being that of the iframe's node document. This means that the client ends up being the settings object of RELAY. Which means we have a bug.

Flags: needinfo?(annevk)

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #3)

This is probably a consequence of us storing the CSP for attributes when those attributes get set, along with the principal. We did that for extensions, but if it's web-visible then we may want to rethink the exact details of how we do it. See also bug 1529877 comment 2.

For example, maybe we should only use the CSP of the stored principal if it's an extension principal or something.

This is what we should already be doing (see bug 1529877 comment 9). I'm not sure what's going on here, but it shouldn't be the result of the add-on CSP overrides unless we're doing something wrong.

If I had to guess, I'd say this has something to do with the iframe being created in the top-level document and then injected into the iframe document (without even an adoptNode() call...).

That really shouldn't cause a problem, but I would not be entirely surprised if it does.

I'm not sure what's going on here

See bug 1529877 comment 10. The frameloader is not checking OverridesCSP() when determining the "csp to inherit" from the triggering principal.

Status: UNCONFIRMED → NEW
Ever confirmed: true

OK, having now read the code in comment 1 more carefully...

This isn't an issue of CSP overrides so much as triggering principals. The top-level document creates an <iframe> and sets its source to a data: URL, which then inherits the stored triggering principal, and also its CSP. So I think this is mainly a question of what we want to do about the content triggering principal in this case.

I didn't add any special casing to the triggering principal logic on the premise that any content principal script that has access to a node has to be same-origin with it anyway. Though perhaps document.domain changes might add some additional corner cases.

The CSP inheritance was one aspect that I hadn't considered, but that should also stop being an issue once content principals stop storing CSPs.

I'm honestly more inclined to have these frames inherit the CSP of the script that created them, but I don't know how that aligns with the spec. I suspect not very well.

sets its source to a data: URL

Sets its src attribute to a data: URL.

which then inherits the stored triggering principal

Which it didn't use to do before we added the CSP overrides, because that thing wasn't stored.

Note that this principal is also used as the CSP for that frame load (modulo the work to move CSP out of principals), so for the extension case we really did want the stored principal as the triggering principal.

but I don't know how that aligns with the spec.

It doesn't.

Again, I think we should just do the OverridesCSP() check in frameloader and only use the stored principal if it does...

(In reply to Boris Zbarsky [:bzbarsky, bz on IRC] from comment #9)

Again, I think we should just do the OverridesCSP() check in frameloader and only use the stored principal if it does...

FWIW, I am trying to incorporate that change within Bug 1532207.

Depends on: 1532207
Priority: -- → P3
Whiteboard: [domsecurity-backlog1]
Severity: normal → S3
You need to log in before you can comment on or make changes to this bug.