Bug 37392

Summary:

Run the SVG <img> rendering context in a unique origin as a defense in depth measure

Product:

WebKit

Reporter:

Chris Evans <scarybeasts>

Component:

SVG

Assignee:

Nobody <webkit-unassigned>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

abarth, commit-queue, darin

Priority:

Version:

528+ (Nightly build)

Hardware:

OS:

OS X 10.5

Attachments:

Description	Flags
Simple defense-in-depth patch.	abarth: review-
Add new bitmap to make forced flags more sturdy :)	none

Chris Evans

Reported 2010-04-10 15:22:54 PDT

Patch forthcoming.

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
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.

Chris Evans

Comment 1 2010-04-10 16:22:16 PDT

Created attachment 53058 [details] Simple defense-in-depth patch.

Chris Evans

Comment 2 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.

Adam Barth

Comment 3 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.

Chris Evans

Comment 4 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.

Adam Barth

Comment 5 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.

Chris Evans

Comment 6 2010-04-10 18:40:16 PDT

Created attachment 53065 [details] Add new bitmap to make forced flags more sturdy :)

Adam Barth

Comment 7 2010-04-10 20:38:27 PDT

Comment on attachment 53065 [details] Add new bitmap to make forced flags more sturdy :) Very nice.

Chris Evans

Comment 8 2010-04-10 20:59:52 PDT

Can you cq+ it? I don't think my WebKit committer paperwork was processed yet ;-)

WebKit Commit Bot

Comment 9 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>

WebKit Commit Bot

Comment 10 2010-04-10 22:17:15 PDT

All reviewed patches have been landed. Closing bug.

Darin Adler

Comment 11 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?

Adam Barth

Comment 12 2010-04-10 23:57:33 PDT

Fixed in http://trac.webkit.org/changeset/57445

Note You need to log in before you can comment on or make changes to this bug.