Bug 92396 – Images loaded via object elements should be governed by the 'object-src' CSP directive.

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Mike West]

The attached test gives results that I don't understand; I think that setting `object-src 'none'` should block the image from being loaded via the `object` tag (see the spec: "It is not required that the consumer of the element's data be a plugin in order for the object-src directive to be enforced. ...").

I'm not sure if the behavior I'm seeing is a bug in my understanding of plugins, or a bug in our CSP implementations (or a bug in my test, I suppose).

[Adam Barth]

It's likely a bug in our implementation.  This is a tricky corner case.

[Mike West]

Created attachment 154685 [details]
Broken test.

[Mike West]

Ok. Then I'll take a look at solving it. At some point. Possibly soon, as this makes it tough to test `plugin-types`.

Any idea where I might want to start looking? :)

[Mike West]

CCing Bernhard and Jochen, who both know things about plugins. :)

[Mike West]

*ahem* Bernhard _and Jochen_.

[Mike West]

Created attachment 160598 [details]
Patch

[Mike West]

The attached patch is a first pass at running image content through CSP even if it's loaded via an object element. I'm not convinced that this is the right place for the check, but I'm not sure where else to put it. ImageLoader seems like the wrong place and anything else is too late. *shrug*

WDYT?

Comment on attachment 160598 [details]
Patch

I believe that this is the wrong place, as this won't catch redirects.

What about adding a callback to ImageLoaderClient to do the CSP checks on redirects?

[Mike West]

(In reply to comment #8)
> (From update of attachment 160598 [details])
> I believe that this is the wrong place, as this won't catch redirects.
> 
> What about adding a callback to ImageLoaderClient to do the CSP checks on redirects?

Hrm. If I'm understanding the layout of things correctly, this would involve switching up some of the CSP checks in CachedResourceLoader::canRequest to call out to the relevant XXXLoaderClient (that is, the various HTMLXXXElement objects), kinda like we did in MainResourceLoader::willSendRequest for 'form-action' a few patches back...

Is that the structure you're thinking of, Jochen?

(In reply to comment #9)
> (In reply to comment #8)
> > (From update of attachment 160598 [details] [details])
> > I believe that this is the wrong place, as this won't catch redirects.
> > 
> > What about adding a callback to ImageLoaderClient to do the CSP checks on redirects?
> 
> Hrm. If I'm understanding the layout of things correctly, this would involve switching up some of the CSP checks in CachedResourceLoader::canRequest to call out to the relevant XXXLoaderClient (that is, the various HTMLXXXElement objects), kinda like we did in MainResourceLoader::willSendRequest for 'form-action' a few patches back...
> 
> Is that the structure you're thinking of, Jochen?

The alternative would be to teach the CachedResourceLoader about images that are loaded via an object tag. Not sure which solution is better.

[Mike West]

(In reply to comment #10)
> (In reply to comment #9)
> > (In reply to comment #8)
> > > (From update of attachment 160598 [details] [details] [details])
> > > I believe that this is the wrong place, as this won't catch redirects.
> > > 
> > > What about adding a callback to ImageLoaderClient to do the CSP checks on redirects?
> > 
> > Hrm. If I'm understanding the layout of things correctly, this would involve switching up some of the CSP checks in CachedResourceLoader::canRequest to call out to the relevant XXXLoaderClient (that is, the various HTMLXXXElement objects), kinda like we did in MainResourceLoader::willSendRequest for 'form-action' a few patches back...
> > 
> > Is that the structure you're thinking of, Jochen?
> 
> The alternative would be to teach the CachedResourceLoader about images that are loaded via an object tag. Not sure which solution is better.

Naah, CachedResourceLoader is complicated enough already. :)

I'll try to fiddle around with the HTMLXXXElement structure sometime this week. Thanks!

[Mike West]

Unassigning myself; let's be realistic about what I'm actually working on. :/

[Radar WebKit Bug Importer]

<rdar://problem/24730219>

[Patrick Griffis]

Fixed by r288792