Closed
Bug 849791
(CVE-2013-1695)
Opened 12 years ago
Closed 12 years ago
Frame DocShells do not inherit sandbox flags from their parents.
Categories
(Core :: DOM: Navigation, defect)
Core
DOM: Navigation
Tracking
()
RESOLVED
FIXED
mozilla22
People
(Reporter: bobowen, Assigned: imelven)
References
Details
(Keywords: sec-low, Whiteboard: [adv-main22+])
Attachments
(2 files)
2.52 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
6.31 KB,
patch
|
bzbarsky
:
review+
|
Details | Diff | Splinter Review |
At the moment the sandbox flags aren't copied to a new frame's DocShell.
So, if someone has a site that other people are hosting in a sandboxed iframe, they can just add a frame to their site and they're suddenly not sandboxed at all.
I believe the problem is in content/base/src/nsFrameLoader.cpp
// Is this an <iframe> with a sandbox attribute or a parent which is
// sandboxed ?
HTMLIFrameElement* iframe =
HTMLIFrameElement::FromContent(mOwnerContent);
uint32_t sandboxFlags = 0;
if (iframe) {
sandboxFlags = iframe->GetSandboxFlags();
uint32_t parentSandboxFlags = iframe->OwnerDoc()->GetSandboxFlags();
if (sandboxFlags || parentSandboxFlags) {
// The child can only add restrictions, not remove them.
sandboxFlags |= parentSandboxFlags;
mDocShell->SetSandboxFlags(sandboxFlags);
}
}
It only checks to see if the parent is sandboxed, if it is an iframe element.
I haven't got time to add a test case at the moment, I'll try and add one later.
Reporter | ||
Updated•12 years ago
|
Blocks: framesandbox
Updated•12 years ago
|
Component: Security → Document Navigation
Assignee | ||
Comment 1•12 years ago
|
||
Marking as sec-low based on other iframe sandbox bugs. Open to discussion, as always.
Keywords: sec-low
Comment 2•12 years ago
|
||
Testcase:
test.html:
<iframe sandbox="allow-same-origin" src="foo.html"></iframe>
foo.html:
<frameset> <frame src="baz.html"> </frameset>
baz.html:
<script> alert('FAIL'); </script>
Assignee | ||
Updated•12 years ago
|
Assignee: nobody → imelven
Assignee | ||
Updated•12 years ago
|
Version: unspecified → Trunk
Assignee | ||
Comment 3•12 years ago
|
||
This fixes bz's test case. I've been working on tests but they're not quite finished yet. I'm trying to avoid making them work like the testcase since then we're checking a script didn't run, which is maybe a bit racey.
Attachment #725030 -
Flags: review?(bzbarsky)
Comment 4•12 years ago
|
||
Comment on attachment 725030 [details] [diff] [review]
patch
r=me
Attachment #725030 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 6•12 years ago
|
||
Comment 7•12 years ago
|
||
Comment on attachment 725104 [details] [diff] [review]
tests
r=me assuming these fail without the patch.
Attachment #725104 -
Flags: review?(bzbarsky) → review+
Assignee | ||
Comment 8•12 years ago
|
||
(In reply to Boris Zbarsky (:bz) from comment #7)
> Comment on attachment 725104 [details] [diff] [review]
> tests
>
> r=me assuming these fail without the patch.
Just verified to make sure they do fail (for me locally at least) without the patch. Thanks for the quick reviews, Boris ! :D
Assignee | ||
Comment 9•12 years ago
|
||
Comment 10•12 years ago
|
||
https://hg.mozilla.org/mozilla-central/rev/629a55eb6ff8
https://hg.mozilla.org/mozilla-central/rev/512c93bfdb66
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Updated•12 years ago
|
status-b2g18:
--- → wontfix
status-firefox21:
--- → wontfix
status-firefox22:
--- → fixed
status-firefox23:
--- → fixed
status-firefox-esr17:
--- → wontfix
Updated•11 years ago
|
Whiteboard: [adv-main22+]
Updated•11 years ago
|
Alias: CVE-2013-1695
Updated•11 years ago
|
Blocks: CVE-2013-5614
Updated•10 years ago
|
Group: core-security
You need to log in
before you can comment on or make changes to this bug.
Description
•