Bug 62257

Summary: Disallow use of cross-domain media (images, video) in WebGL
Product: WebKit Reporter: Kenneth Russell <kbr>
Component: WebGLAssignee: Kenneth Russell <kbr>
Status: RESOLVED FIXED    
Severity: Normal CC: abarth, cmarrin, zmo
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 61015, 62368    
Bug Blocks:    
Attachments:
Description Flags
Patch abarth: review+

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+
DetailsFormatted DiffDiff
View All Add attachment proposed patch, testcase, etc.
Kenneth Russell
Comment 1 2011-06-07 19:34:31 PDT
Created attachment 96360 [details] Patch
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
Committed r88387: <http://trac.webkit.org/changeset/88387>
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
Committed r88489: <http://trac.webkit.org/changeset/88489>
Note You need to log in before you can comment on or make changes to this bug.