Bug 100635

Summary: Block SVG external references pending a security review
Product: WebKit Reporter: Adam Barth <abarth>
Component: SVGAssignee: Adam Barth <abarth>
Status: RESOLVED FIXED    
Severity: Normal CC: eric, inferno, japhet, krit, ossy, senorblanco, thorton, webkit.review.bot, zimmermann
Priority: P2 Keywords: InRadar, WebExposed
Version: 528+ (Nightly build)   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch
none
Patch for landing none

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.