iframe CSP bypass via "unsafe-inline" script in parent frame despite iframe CSP having `default-src: none`
Categories
(Core :: DOM: Security, defect, P3)
Tracking
()
People
(Reporter: su3604, Assigned: tschuster)
References
(Blocks 1 open bug)
Details
(Keywords: sec-moderate, Whiteboard: [adv-main122+][adv-esr115.7+])
Attachments
(3 files)
User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_15_7) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/100.0.4896.75 Safari/537.36
Steps to reproduce:
- Set the following files on your web server.
<html>
<head>
<meta http-equiv="Content-Security-Policy" content="script-src 'unsafe-inline'">
</head>
<body>
<iframe id=x src='self.html'></iframe>
<script nonce=123>
x.onload=_=>x.contentDocument.write('<script>alert(1)<\/script>');
</script>
</body>
</html>
<html>
<head>
<meta http-equiv="Content-Security-Policy" content="default-src 'none'">
</head>
<body>
</body>
</html>
- Go to the test.html page
Actual results:
The inline script written in the iframe is executed.
Expected results:
CSP is defined for each page. That is, when specifying a separate page (eg, self.html) other than the local scheme (https://www.w3.org/TR/CSP3/#security-inherit-csp) in the iframe src, the iframe must be affected by the corresponding CSP, not the parent CSP. However, I confirmed that the CSP of the parent page is inherited to another separate page when writing is performed by calling the x.contentDocument.write function in Firefox.
Although self.html should not allow inline script execution according to its CSP default-src 'none', an attacker can use the trick in this report to cause inline scripts to be executed in self.html environment.
|
Reporter | |
Comment 1•3 years ago
|
||
FYI, I've seen Chrome, Edge, and Safari correctly enforce the CSP of the child page in this case.
|
||
Updated•3 years ago
|
|
||
Updated•3 years ago
|
|
||
Comment 2•3 years ago
|
||
I'll take a couple of minutes later this week to better understand how and why the CSP is inherited.
|
|
||
Comment 3•3 years ago
|
||
If the parent page had reached in and added an inline onclick handler or whatever, that should get blocked.
If the CSP were in the HTTP header it should have stuck around.
I think in this case (have not experimented yet), the document.write() implies a document.open() which blows away the current document (where the CSP was set in a META tag) and then you're left without a CSP. Maybe it would inherit from the parent, but that allows unsafe-ineline.
Freddy will investigate
|
|
||
Updated•3 years ago
|
|
|
||
Comment 4•3 years ago
•
|
||
OK, I took a moment to look at this. To reproduce this, I have modified the child document to have a big headline (<h1>Child</h1>) would suffice.
Loading the test case will verify Dan's assumption in comment 3: The self.html document will be thrown away completely and the document.write() call creates a new one (the headline goes away). The new document doesn't have a <meta> CSP and the script will be allowed.
Contrary, if the parent script did the following instead:
s = x.contentDocument.createElement("script");
s.innerText = "alert(2)";
x.contentDocument.body.append(s);
Then the <meta> element would still be intact and therefore disallow the inline script:
Content Security Policy: The page’s settings blocked the loading of a resource at inline (“default-src”).
I think we can fix this as INVALID, but I'm happy to hear objections.
|
|
||
Comment 5•3 years ago
|
||
(In reply to Frederik Braun [:freddy] from comment #4)
I think we can fix this as INVALID, but I'm happy to hear objections.
(In reply to Seongil Wi from comment #1)
FYI, I've seen Chrome, Edge, and Safari correctly enforce the CSP of the child page in this case.
I'm curious why other browsers would behave differently here, if the spec says we're doing the right thing?
|
|
||
Comment 6•2 years ago
|
||
Huh, it looks like the CSP should be copied over but never is. Or does it copy it from the opener (instead of from the previous popup document?)
Maybe that changed with recent document/navigation changes?
https://searchfox.org/mozilla-central/rev/b0844a16d60042c8e1e8f50465a89c1696d283fb/dom/base/Document.cpp#9535-9544
I'll keep the needinfo to continue investigating. Sorry this has been taking so long.
I'm also CCing Tom who has done most of the recent CSP work. Not necessarily because I expect him to pick this up, but because I'm sure this will be interesting to him :)
|
|
||
Comment 7•2 years ago
|
||
I believe this is a dupe of bug 1754872, where both Mike West and Anne think the spec supports the Chrome behavior. document.open() is not creating an actual fresh "about:blank" document. They interpret https://html.spec.whatwg.org/multipage/dynamic-markup-insertion.html#document-open-steps as emptying out the document but then re-using it rather than creating a new document. That would preserve the "policy container" attributes including CSP.
|
||
Comment 8•1 year ago
|
||
Similarly to what is happening in Bug 1754872, when initially parsing and binding the self.html iframe's document we parse the Meta tag CSP directives and apply them to the iframe's document.
Later, when we are in the implicit document.open we inherit the opening document's CSP directives. We replace the existing DOM in the iframe's document with the <script> tag passed into the subsequent document.write call and when binding the <script> tag to the tree call CSPAllowsInlineScript which in turn calls nsCSPContext::GetAllowsInline.
Here mPolicies has a single entry, which is the inherited script-src 'unsafe-inline' directive from the opening document, but doesn't retain the Meta tag CSP directive previously assigned from self.html so ultimately allows the script to run.
Based on both of these bugs and their test cases and comments it seems like inheriting the CSP directives from the opener isn't what we actually want for a same-origin iframe and that we should be inheriting the iframe's CSP directives from before calling document.open? Or is the current behavior what we want and compliant with the spec?
I'm not familiar with any of the relevant specs, however, so I'm needinfoing Tom S. and Christoph to help me with guidance. I'm also attaching the test case provided by Seongil as a mochitest to help debug if needed
|
|
||
Comment 9•1 year ago
|
||
|
|
||
Comment 10•1 year ago
|
||
right, we currently, incorrectly, treat document.write() on an existing document (or more specifically, the implicit document.open() that happens on the first write) as if we navigated to "about:blank" and built a new document from scratch. At least, from the CSP and probably sandbox POV, if not in all HTML/DOM particulars.
These rules will apply to any two existing same-origin documents where a script running in one (AD for active document) modifies the other (TD for target document) through a window reference. Could be a parent document and an iframe, a window and a popup it opened, or a popup writing back into its opener. The Target Document will have policies associated with it, in this case we're talking about CSP, and those existing policies should be preserved; it should not inherit ANY policy from the AD that re-wrote it using document.write(), just as it wouldn't inherit anything if AD modified TD through pure DOM node deletion/creation.
If the newly written content has a <meta CSP> in it that gets parsed then that should be added to the list of TD policies, but we should not remove or replace any that already existed even if they were from <meta> tags instead of headers. If we allow document.write() to erase the policy we created from previous <meta> tag content then this could be used by a document to relax it's own CSP.
|
|
||
Comment 11•1 year ago
|
||
Our Document::Open() code that you reference in comment 8 has comments aligning each bit of code with a step of the spec -- except the bit you pointed to! Clearly that was added to solve some inheritance problem we were having, but it's causing this one. We can't simply remove it without figuring out why it was there initially. Is this where we inherit policies because fresh "about:blank" documents happen to go through here too? If so it should be moved to some earlier document creation time. Or maybe we've done it there, too, by now and this is "extra"?
Likewise, the placement of this securityInfo bit is suspicious:
https://searchfox.org/mozilla-central/rev/1865e9fba4166ab2aa6c9d539913115723d9cc53/dom/base/Document.cpp#9806-9808
That, too, looks like something that should happen on document creation, not document.open(), and not get reset when manipulated from the outside. I think it's not harmful here since this can only happen in a same-origin scenario, but I can imagine some edge-case scenarios where the values would be technically wrong (but not meaningfully so). for example: Long-lived tab where, after putting your laptop to sleep and commuting home, you interact with framed content in a way that causes a same-site navigation or reload. Maybe one of those locations uses a MITM security proxy (Enterprise middlebox, or home anti-virus scanning) so you'd get a different certificate chain and ciphersuite details. Wouldn't be that important -- presumably you'd get the same values for the parent page if you had refreshed it.
Mostly I mention it because it seems to come from the same mental-model as the CSP inheritance problem we're having, and may mean there wasn't a better place for it at the time. Hopefully there is now, but if not you may need to create one.
|
|
||
Comment 12•1 year ago
|
||
I've updated the patch to remove that block of code in Document::Open which seems to most likely have been code that either got duplicated or inadvertently left behind in past refactors of the CSP code. I believe it was once doing what this code block is doing now, although trying to track it down past the changes from Bug 965637 has been near impossible.
I've also tested these changes out pretty extensively on try which only came back with known intermittents. The last thing I need to do is write some more comprehensive tests and then this will be ready for review! Thanks Dan for your help, I greatly appreciate it
|
Assignee | |
Comment 13•1 year ago
|
||
I wasn't really aware of how document.write works (yuck!). It seems a bit unfortunate that we are going to start relying on that apparently non-standard behavior with about:blank, which is what makes CSP_ShouldResponseInheritCSP return true.
|
||
Updated•1 year ago
|
|
|
||
Updated•1 year ago
|
|
|
||
Comment 14•1 year ago
|
||
It seems a bit unfortunate that we are going to start relying on that apparently non-standard behavior with about:blank
document.open()/write() is historical behavior that the WHATWG has very much attempted to document/standardize.
|
|
||
Comment 15•1 year ago
|
||
It seems this is reviewed and ready to go. I don't recall what was keeping us from landing? Maybe a green try run?
Can you kick this over the finish line, Tom?
|
|
Assignee | |
Updated•1 year ago
|
|
|
Assignee | |
Comment 16•1 year ago
|
||
Looks like the tests need to be updated still.
|
|
||
Updated•1 year ago
|
|
||
Comment 18•1 year ago
|
||
|
||
Comment 19•1 year ago
•
|
||
|
||
Updated•1 year ago
|
|
||
Updated•1 year ago
|
|
|
Assignee | |
Comment 21•1 year ago
|
||
|
|
||
Comment 22•1 year ago
|
||
Any concerns with an ESR approval request?
115.7esr builds next week.
|
|
Assignee | |
Comment 23•1 year ago
|
||
Comment on attachment 9371286 [details]
Bug 1764343 - ESR115: Remove steps in Document::Open not derived from spec. r?dveditz
ESR Uplift Approval Request
- If this is not a sec:{high,crit} bug, please state case for ESR consideration: CSP is not properly being applied and could allow (among others) unexpected script execution on websites.
- User impact if declined:
- Fix Landed on Version: 122
- Risk to taking this patch: Low
- Why is the change risky/not risky? (and alternatives if risky): On Beta already and matches (tested) Chrome behavior.
|
|
||
Comment 24•1 year ago
|
||
Comment on attachment 9371286 [details]
Bug 1764343 - ESR115: Remove steps in Document::Open not derived from spec. r?dveditz
Approved for 115.7esr.
|
|
||
Comment 25•1 year ago
|
||
| uplift | ||
|
|
||
Updated•1 year ago
|
|
||
Updated•1 year ago
|
|
|
||
Comment 26•1 year ago
|
||
|
||
Updated•1 year ago
|
|
|
||
Comment 27•9 months ago
|
||
Making Firefox 122 security bugs public. [bugspam filter string: Pilgarlic-Towers]
Description
•