Bug 37392 - Run the SVG <img> rendering context in a unique origin as a defense in depth measure
Summary: Run the SVG <img> rendering context in a unique origin as a defense in depth ...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC OS X 10.5
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2010-04-10 15:22 PDT by Chris Evans
Modified: 2010-04-10 23:57 PDT (History)
3 users (show)

See Also:


Attachments
Simple defense-in-depth patch. (2.05 KB, patch)
2010-04-10 16:22 PDT, Chris Evans
abarth: review-
Details | Formatted Diff | Diff
Add new bitmap to make forced flags more sturdy :) (3.23 KB, patch)
2010-04-10 18:40 PDT, Chris Evans
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Chris Evans 2010-04-10 15:22:54 PDT
Patch forthcoming.
Comment 1 Chris Evans 2010-04-10 16:22:16 PDT
Created attachment 53058 [details]
Simple defense-in-depth patch.
Comment 2 Chris Evans 2010-04-10 16:22:48 PDT
I've not found a concrete problem due to this, but the area makes me nervous enough to have created this patch.
Comment 3 Adam Barth 2010-04-10 16:51:57 PDT
Comment on attachment 53058 [details]
Simple defense-in-depth patch.

That method on FrameLoader is very sketchy.  Can't we operate on the SecurityOrigin directly?  That won't be overridden in any funny ways.
Comment 4 Chris Evans 2010-04-10 17:25:20 PDT
The only way I can see for the sandbox flags on the FrameLoader to get overridden is if a parent HTMLFrameElement changes state. This will never happen to our temporary SVG loader, which has an SVG document at the root :)

I like doing this on the FrameLoader because any other origins created from it will also inherit the same sandboxing state. For example, an SVG could contain an html:iframe element[*]. If we do a one-off patch up on the SecurityOrigin, I worry that a complicated & clever SVG could bypass it.

[*] - this is pretty ugly in the <img> context. I may be changing things here, but for now we should assume these uglies exist.
Comment 5 Adam Barth 2010-04-10 17:27:23 PDT
Ah good, point.  I'm worried that other consumers of this API will get confused.  Can we make this a new bit field that always gets applied to the "natural" sandbox policy?  That way it could survive updates.
Comment 6 Chris Evans 2010-04-10 18:40:16 PDT
Created attachment 53065 [details]
Add new bitmap to make forced flags more sturdy :)
Comment 7 Adam Barth 2010-04-10 20:38:27 PDT
Comment on attachment 53065 [details]
Add new bitmap to make forced flags more sturdy :)

Very nice.
Comment 8 Chris Evans 2010-04-10 20:59:52 PDT
Can you cq+ it? I don't think my WebKit committer paperwork was processed yet ;-)
Comment 9 WebKit Commit Bot 2010-04-10 22:17:10 PDT
Comment on attachment 53065 [details]
Add new bitmap to make forced flags more sturdy :)

Clearing flags on attachment: 53065

Committed r57438: <http://trac.webkit.org/changeset/57438>
Comment 10 WebKit Commit Bot 2010-04-10 22:17:15 PDT
All reviewed patches have been landed.  Closing bug.
Comment 11 Darin Adler 2010-04-10 23:13:52 PDT
I suggest calling these "forced sandbox flags" in the code, not "force sandbox flags". Maybe the names you ended up using in the patch were a typo?
Comment 12 Adam Barth 2010-04-10 23:57:33 PDT
Fixed in http://trac.webkit.org/changeset/57445