Bug 62257 - Disallow use of cross-domain media (images, video) in WebGL
: Disallow use of cross-domain media (images, video) in WebGL
Status: RESOLVED FIXED
: WebKit
WebGL
: 528+ (Nightly build)
: All All
: P2 Normal
Assigned To:
:
:
: 61015 62368
:
  Show dependency treegraph
 
Reported: 2011-06-07 19:19 PST by
Modified: 2011-06-09 15:02 PST (History)


Attachments
Patch (39.02 KB, patch)
2011-06-07 19:34 PST, Kenneth Russell
abarth: review+
Review Patch | Details | Formatted Diff | Diff


Note

You need to log in before you can comment on or make changes to this bug.


Description From 2011-06-07 19:19:45 PST
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/
------- Comment #1 From 2011-06-07 19:34:31 PST -------
Created an attachment (id=96360) [details]
Patch
------- Comment #2 From 2011-06-07 19:57:44 PST -------
(From update of attachment 96360 [details])
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.  :)
------- Comment #3 From 2011-06-08 14:13:38 PST -------
(In reply to comment #2)
> (From update of attachment 96360 [details] [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.
------- Comment #4 From 2011-06-08 14:28:00 PST -------
Committed r88387: <http://trac.webkit.org/changeset/88387>
------- Comment #5 From 2011-06-09 10:37:27 PST -------
Reopening because Ossy rolled out the patch.
------- Comment #6 From 2011-06-09 15:01:51 PST -------
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.
------- Comment #7 From 2011-06-09 15:02:31 PST -------
Committed r88489: <http://trac.webkit.org/changeset/88489>