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)

defect
Not set
normal

Tracking

()

RESOLVED FIXED
mozilla22
Tracking Status
firefox21 --- wontfix
firefox22 --- fixed
firefox23 --- fixed
firefox-esr17 --- wontfix
b2g18 --- wontfix

People

(Reporter: bobowen, Assigned: imelven)

References

Details

(Keywords: sec-low, Whiteboard: [adv-main22+])

Attachments

(2 files)

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.
Blocks: framesandbox
Component: Security → Document Navigation
Marking as sec-low based on other iframe sandbox bugs. Open to discussion, as always.
Keywords: sec-low
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: nobody → imelven
Version: unspecified → Trunk
Attached patch patchSplinter Review
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)
Attachment #725030 - Flags: review?(bzbarsky) → review+
Attached patch testsSplinter Review
Tests.
Attachment #725104 - Flags: review?(bzbarsky)
Comment on attachment 725104 [details] [diff] [review] tests r=me assuming these fail without the patch.
Attachment #725104 - Flags: review?(bzbarsky) → review+
(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
Status: NEW → RESOLVED
Closed: 12 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla22
Whiteboard: [adv-main22+]
Alias: CVE-2013-1695
Group: core-security
You need to log in before you can comment on or make changes to this bug.

Attachment

General

Created:
Updated:
Size: