[webkit-reviews] review requested: [Bug 21288] Implement HTML5's sandbox attribute for iframes : [Attachment 44069] Slightly revised patch (added defensive FrameLoader initialization)

bugzilla-daemon at webkit.org bugzilla-daemon at webkit.org
Tue Dec 1 05:09:28 PST 2009


Patrik Persson <patrik.j.persson at ericsson.com> has asked  for review:
Bug 21288: Implement HTML5's sandbox attribute for iframes
https://bugs.webkit.org/show_bug.cgi?id=21288

Attachment 44069: Slightly revised patch (added defensive FrameLoader
initialization)
https://bugs.webkit.org/attachment.cgi?id=44069&action=review

------- Additional Comments from Patrik Persson <patrik.j.persson at ericsson.com>
(In reply to comment #59)
> So the correct sandbox flags is "all" in a case like this? Can that really be

> right? Or should the sandbox flags somehow be inherited from the frame that's

> doing the creating?

Copying the flags from the creating frame would probably be safe.
However, there are quite many locations where SecurityOrigins are
instantiated (about 20 sites, 10 classes in different parts of
WebCore).  Each of these locations would need modification to locate
the creating frame, and I don't currently know of a common way to do
that.

I'm not sure there is any added safety in this anyway.	Sandbox flags
have no meaning without a frame context (in my interpretation of HTML5
at least).

I propose we leave this out of the patch, to avoid against adding this
complexity for (limited) defensive purposes.  Defensive initial values
could help us catch bugs, but this complexity could help create them
too.

I still think your proposal for defensive initialization in the
FrameLoader makes good sense.  I'm attaching a slightly revised patch
with that included.


More information about the webkit-reviews mailing list