[webkit-reviews] review denied: [Bug 38424] add support for text/html-sandboxed on sandboxed iframes : [Attachment 66200] Yet another build fix

[bugzilla-daemon at webkit.org]

Adam Barth <abarth at webkit.org> has denied Patrik Persson
<patrik.j.persson at ericsson.com>'s request for review:
Bug 38424: add support for text/html-sandboxed on sandboxed iframes
https://bugs.webkit.org/show_bug.cgi?id=38424

Attachment 66200: Yet another build fix
https://bugs.webkit.org/attachment.cgi?id=66200&action=review

------- Additional Comments from Adam Barth <abarth at webkit.org>
View in context:
https://bugs.webkit.org/attachment.cgi?id=66200&action=prettypatch

We definitely need some more tests.  For example, you should test the case
where you load a document into a sandboxed frame, remove the sandbox attribute,
and then try to load a text/html-sandboxed resource (and the reverse).	Those
tests will show that we're getting the bit from the frame and not from the
document (which freezes its bits).  Also, we should think about how to test for
the absence of the gap mentioned below.

> WebCore/loader/DocumentLoader.cpp:284
> +	       && !frameLoader->isSandboxed(SandboxOrigin)
I wonder slightly if this is the right time to ask the sandbox state.  We need
to be sure the frame as the sandbox bit turned on at the instant the document
freezes it's sandbox bits.  If there's a window of opportunity for JavaScript
to change the frame's attribute, we could get in trouble, especially since that
scenario can be triggered by an attacker.  How can we assure ourselves that
there's no gap here?