RESOLVED FIXED 62257
Disallow use of cross-domain media (images, video) in WebGL
https://bugs.webkit.org/show_bug.cgi?id=62257
Summary Disallow use of cross-domain media (images, video) in WebGL
Kenneth Russell
Reported 2011-06-07 19:19:45 PDT
Due to the possibility of using WebGL to examine the contents of cross-origin images [1], it is necessary to disallow the use of cross-origin media in WebGL. The WebGL specification and conformance suite have already been updated to mandate this new behavior, and the HTML specification and WebKit implementation have been updated to allow applications to use CORS to request permission to use cross-domain media. [1] http://www.khronos.org/webgl/security/
Attachments
Patch (39.02 KB, patch)
2011-06-07 19:34 PDT, Kenneth Russell
abarth: review+
Kenneth Russell
Comment 1 2011-06-07 19:34:31 PDT
Adam Barth
Comment 2 2011-06-07 19:57:44 PDT
Comment on attachment 96360 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=96360&action=review Looks great. Thanks. > Source/WebCore/html/canvas/CanvasRenderingContext.cpp:103 > + m_cleanOrigins.add(url.string()); m_cleanOrigins => m_cleanURLs since this hold URLs and not origins? > Source/WebCore/html/canvas/CanvasRenderingContext.cpp:130 > +void CanvasRenderingContext::checkOrigin(const CanvasPattern* pattern) > +{ > + if (wouldTaintOrigin(pattern)) > + canvas()->setOriginTainted(); > +} > + > +void CanvasRenderingContext::checkOrigin(const HTMLCanvasElement* sourceCanvas) > +{ > + if (wouldTaintOrigin(sourceCanvas)) > + canvas()->setOriginTainted(); > +} > + > +void CanvasRenderingContext::checkOrigin(const HTMLImageElement* image) > +{ > + if (wouldTaintOrigin(image)) > + canvas()->setOriginTainted(); > +} > + > +void CanvasRenderingContext::checkOrigin(const HTMLVideoElement* video) > +{ > + if (wouldTaintOrigin(video)) > + canvas()->setOriginTainted(); > +} This feel ripe for a template. > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2812 > + UNUSED_PARAM(ec); You can just remove the name of the ec parameter instead of using this macro. > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2815 > + // It is no longer possible to taint the origin using the WebGL API. I'd remove this comment. That's going to read strange in two years. :)
Kenneth Russell
Comment 3 2011-06-08 14:13:38 PDT
(In reply to comment #2) > (From update of attachment 96360 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=96360&action=review > > Looks great. Thanks. Thanks hugely to you and Nate for doing the CORS support for images, so that this patch won't completely break certain WebGL apps. :) > > Source/WebCore/html/canvas/CanvasRenderingContext.cpp:103 > > + m_cleanOrigins.add(url.string()); > > m_cleanOrigins => m_cleanURLs since this hold URLs and not origins? OK, I'll rename this member in the code I'll land. > > Source/WebCore/html/canvas/CanvasRenderingContext.cpp:130 > > +void CanvasRenderingContext::checkOrigin(const CanvasPattern* pattern) > > +{ > > + if (wouldTaintOrigin(pattern)) > > + canvas()->setOriginTainted(); > > +} > > + > > +void CanvasRenderingContext::checkOrigin(const HTMLCanvasElement* sourceCanvas) > > +{ > > + if (wouldTaintOrigin(sourceCanvas)) > > + canvas()->setOriginTainted(); > > +} > > + > > +void CanvasRenderingContext::checkOrigin(const HTMLImageElement* image) > > +{ > > + if (wouldTaintOrigin(image)) > > + canvas()->setOriginTainted(); > > +} > > + > > +void CanvasRenderingContext::checkOrigin(const HTMLVideoElement* video) > > +{ > > + if (wouldTaintOrigin(video)) > > + canvas()->setOriginTainted(); > > +} > > This feel ripe for a template. Agreed; replaced with one. > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2812 > > + UNUSED_PARAM(ec); > > You can just remove the name of the ec parameter instead of using this macro. Done. > > Source/WebCore/html/canvas/WebGLRenderingContext.cpp:2815 > > + // It is no longer possible to taint the origin using the WebGL API. > > I'd remove this comment. That's going to read strange in two years. :) Agreed it will read strangely. :) I've rewritten rather than removed it, though.
Kenneth Russell
Comment 4 2011-06-08 14:28:00 PDT
Adam Barth
Comment 5 2011-06-09 10:37:27 PDT
Reopening because Ossy rolled out the patch.
Kenneth Russell
Comment 6 2011-06-09 15:01:51 PDT
I looked at the failures on the three affected platforms (Leopard, GTK and Qt) and the reason the new tests were failing there is that WebGL isn't supported on those ports. I'm recommitting the original patch with new Skipped entries for those three ports.
Kenneth Russell
Comment 7 2011-06-09 15:02:31 PDT
Note You need to log in before you can comment on or make changes to this bug.