Disallow use of cross-domain media (images, video) in WebGL

• Component: WebGL
• Status: RESOLVED FIXED

[Kenneth Russell]

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/

[Kenneth Russell]

Created attachment 96360 [details]
Patch

[Adam Barth]

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]

(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]

Committed r88387: <http://trac.webkit.org/changeset/88387>

[Adam Barth]

Reopening because Ossy rolled out the patch.

[Kenneth Russell]

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]

Committed r88489: <http://trac.webkit.org/changeset/88489>