Bug 100635

Summary:

Block SVG external references pending a security review

Product:

WebKit

Reporter:

Adam Barth <abarth>

Component:

SVG

Assignee:

Adam Barth <abarth>

Status:

RESOLVED FIXED

Severity:

Normal

CC:

eric, inferno, japhet, krit, ossy, senorblanco, thorton, webkit.review.bot, zimmermann

Priority:

Keywords:

InRadar, WebExposed

Version:

528+ (Nightly build)

Hardware:

Unspecified

OS:

Unspecified

Attachments:

Description	Flags
Patch	none
Patch	none
Patch for landing	none

Adam Barth

Reported 2012-10-29 00:09:11 PDT

Block SVG external references in the Chromium port

Attachments
Patch (2.70 KB, patch) 2012-10-29 00:10 PDT, Adam Barth	no flags	Details Formatted Diff Diff
Patch (8.67 KB, patch) 2012-10-29 12:59 PDT, Adam Barth	no flags	Details Formatted Diff Diff
Patch for landing (8.22 KB, patch) 2012-10-29 13:43 PDT, Adam Barth	no flags	Details Formatted Diff Diff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.

Adam Barth

Comment 1 2012-10-29 00:10:45 PDT

Created attachment 171165 [details] Patch

Adam Barth

Comment 2 2012-10-29 00:11:34 PDT

I expect that this will cause some tests to fail. I haven't tested locally.

Dirk Schulze

Comment 3 2012-10-29 00:26:18 PDT

Comment on attachment 171165 [details] Patch r=me

Eric Seidel (no email)

Comment 4 2012-10-29 00:35:35 PDT

Comment on attachment 171165 [details] Patch I would have phrased this the other way, and made the define = 0 in Platform.h. Or just turned it off for everyone if we're really concerned.

Eric Seidel (no email)

Comment 5 2012-10-29 00:36:20 PDT

Sorry, I would have re-phrased the ENABLE in the positive as well. ENABLE_SVG_EXTERNAL_RESOURCES. The naming doesn't really matter that much. It also depends on how long we plan to keep it off. :)

Adam Barth

Comment 6 2012-10-29 00:43:09 PDT

Comment on attachment 171165 [details] Patch Ok. I'll flip around the enable. Apparently the spec is going through a security review now. krit is going to look in the WebAppSec working group. I suspect the net result is that we're going to want to use CORS for these loads.

Adam Barth

Comment 7 2012-10-29 00:43:42 PDT

s/look/loop/

Alexey Proskuryakov

Comment 8 2012-10-29 09:49:55 PDT

<rdar://problem/12591955>

Adam Barth

Comment 9 2012-10-29 12:59:09 PDT

Created attachment 171295 [details] Patch

Eric Seidel (no email)

Comment 10 2012-10-29 13:40:33 PDT

Comment on attachment 171295 [details] Patch Is there a timeline for this review?

Adam Barth

Comment 11 2012-10-29 13:41:49 PDT

> Is there a timeline for this review? I don't think krit has emailed security@chromium.org yet, but it will likely go in the review queue when he does.

Adam Barth

Comment 12 2012-10-29 13:43:43 PDT

Created attachment 171302 [details] Patch for landing

WebKit Review Bot

Comment 13 2012-10-29 14:23:28 PDT

Comment on attachment 171302 [details] Patch for landing Clearing flags on attachment: 171302 Committed r132849: <http://trac.webkit.org/changeset/132849>

WebKit Review Bot

Comment 14 2012-10-29 14:23:33 PDT

All reviewed patches have been landed. Closing bug.

Csaba Osztrogonác

Comment 15 2012-10-29 22:55:16 PDT

(In reply to comment #13) > (From update of attachment 171302 [details]) > Clearing flags on attachment: 171302 > > Committed r132849: <http://trac.webkit.org/changeset/132849> ... and a fix landed in http://trac.webkit.org/changeset/132869 without any reference to the original bug and/or revision.

Tim Horton

Comment 16 2012-12-15 14:10:44 PST

For future reference, these appear to have been re-enabled in http://trac.webkit.org/changeset/133538.

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