Bug 100635 - Block SVG external references pending a security review
Summary: Block SVG external references pending a security review
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: SVG (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Adam Barth
URL:
Keywords: InRadar, WebExposed
Depends on:
Blocks:
 
Reported: 2012-10-29 00:09 PDT by Adam Barth
Modified: 2012-12-15 14:10 PST (History)
9 users (show)

See Also:

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 an attachment (proposed patch, testcase, etc.)

Note You need to log in before you can comment on or make changes to this bug.
Description Adam Barth 2012-10-29 00:09:11 PDT 
Block SVG external references in the Chromium port
Comment 1 Adam Barth 2012-10-29 00:10:45 PDT 
Created attachment 171165 [details]
Patch
Comment 2 Adam Barth 2012-10-29 00:11:34 PDT 
I expect that this will cause some tests to fail.  I haven't tested locally.
Comment 3 Dirk Schulze 2012-10-29 00:26:18 PDT 
Comment on attachment 171165 [details]
Patch

r=me
Comment 4 Eric Seidel (no email) 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.
Comment 5 Eric Seidel (no email) 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. :)
Comment 6 Adam Barth 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.
Comment 7 Adam Barth 2012-10-29 00:43:42 PDT 
s/look/loop/
Comment 8 Alexey Proskuryakov 2012-10-29 09:49:55 PDT 
<rdar://problem/12591955>
Comment 9 Adam Barth 2012-10-29 12:59:09 PDT 
Created attachment 171295 [details]
Patch
Comment 10 Eric Seidel (no email) 2012-10-29 13:40:33 PDT 
Comment on attachment 171295 [details]
Patch

Is there a timeline for this review?
Comment 11 Adam Barth 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.
Comment 12 Adam Barth 2012-10-29 13:43:43 PDT 
Created attachment 171302 [details]
Patch for landing
Comment 13 WebKit Review Bot 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>
Comment 14 WebKit Review Bot 2012-10-29 14:23:33 PDT 
All reviewed patches have been landed.  Closing bug.
Comment 15 Csaba Osztrogonác 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.
Comment 16 Tim Horton 2012-12-15 14:10:44 PST 
For future reference, these appear to have been re-enabled in http://trac.webkit.org/changeset/133538.