1246578 - Reassess use of contentaccessible=true for marquee and potentially other UA widgets

Status: NEW

(Core :: DOM: Security, defect, P5)

Description

[:Gijs (he/him)]

Per bug 1245681 comment 28, we should re-examine whether this is really the best way to deal with the issue of the AsyncOpen2 conversions. AFAICT the only things that got added were from bug 1195162 for the marquee and pluginproblem bindings, which involves XBL. Whee.

(which also makes me wonder how we're currently dealing with scrollbar bindings...)

Christoph, can you check that that's all of them?

Comment 1

[Boris Zbarsky [:bzbarsky]]

What set of things did we end up making contentaccessible to make marquee and pluginproblem exposed?  If we restricted to just those two files, it's probably OK.

Comment 2

[Christoph Kerschbaumer [:ckerschb]]

(In reply to Boris Zbarsky [:bz] from comment #1)
> What set of things did we end up making contentaccessible to make marquee
> and pluginproblem exposed?  If we restricted to just those two files, it's
> probably OK.

We moved the pluginproblem out of mozapps/, created pluginproblem/ and made that contentaccessible, see:
https://bugzilla.mozilla.org/attachment.cgi?id=8660815&action=diff

Comment 3

[:Gijs (he/him)]

Is there contentaccessible stuff left post-UA-widget-conversion? If not we should just nuke that or does the UA widget stuff need it to be contentaccessible?

Comment 4

[Brian Grinstead [:bgrins]]

(In reply to :Gijs (he/him) from comment #3)

Is there contentaccessible stuff left post-UA-widget-conversion? If not we should just nuke that or does the UA widget stuff need it to be contentaccessible?

I'm not sure. It looks like they are still registered as such, but I don't know if they need to be anymore. For instance, videocontrols.css is loaded in a UA root but AFAICT isn't set as contentaccessible: https://searchfox.org/mozilla-central/rev/89235894292ac4ab4bd7dac44bc60c2e051905f4/toolkit/themes/shared/jar.inc.mn#100.

• pluginProblemContent.css appears to still be loaded as contentaccessible https://searchfox.org/mozilla-central/rev/89235894292ac4ab4bd7dac44bc60c2e051905f4/toolkit/pluginproblem/jar.mn#6 and we load the CSS files in a UA Shadow Root at https://searchfox.org/mozilla-central/rev/89235894292ac4ab4bd7dac44bc60c2e051905f4/toolkit/content/widgets/pluginProblem.js#30

• marquee does load marquee.css in a UA Shadow Root: https://searchfox.org/mozilla-central/rev/89235894292ac4ab4bd7dac44bc60c2e051905f4/toolkit/content/widgets/marquee.js#362. Here's where it's registered: https://searchfox.org/mozilla-central/rev/89235894292ac4ab4bd7dac44bc60c2e051905f4/toolkit/content/jar.mn#95. Does the "content global %content/global/ contentaccessible=yes" on line 2 in that jar file mean that anything in the jar is contentaccessible?

• pluginproblem and videocontrols both load dtd files so that may be something to look into as well.

FWIW the CSS files for marquee and for pluginproblem look like they could be trivially be inlined into a <style> tag into the UA Shadow Roots if it would help. The videocontrols css file loads subresources so I'm not sure, bug as I mention above it appears that isn't marked as contentaccessible currently.

Comment 5

[:Gijs (he/him)]

(In reply to (Mostly unavailable in July) Brian Grinstead [:bgrins] from comment #4)

(In reply to :Gijs (he/him) from comment #3)

Is there contentaccessible stuff left post-UA-widget-conversion? If not we should just nuke that or does the UA widget stuff need it to be contentaccessible?

I'm not sure. It looks like they are still registered as such, but I don't know if they need to be anymore. For instance, videocontrols.css is loaded in a UA root but AFAICT isn't set as contentaccessible: https://searchfox.org/mozilla-central/rev/89235894292ac4ab4bd7dac44bc60c2e051905f4/toolkit/themes/shared/jar.inc.mn#100.

But it is, because if the content package is contentaccessible, so is the skin package, and as you noted:

"content global %content/global/ contentaccessible=yes" on line 2 in that jar file mean that anything in the jar is contentaccessible?

Not quite, it means anything in chrome://global/content/ is marked contentaccessible (ditto for chrome://global/skin/), which is how videocontrols.css is loaded...

Although videocontrols isn't an example of this working, if I remove the contentaccessible marking for pluginproblem and rebuild, it seems like things work fine (presumably because the UA widgets are slightly more privileged, or something?). Boris, does that sound right? Can we just avoid marking UA widget stylesheets contentaccessible everywhere?

Comment 6

[Boris Zbarsky [:bzbarsky]]

I don't actually know what the security model is for UA widget bits... I'd have to dig through to see what principals are being passed to the loads there.

I can probably do that if someone gives me steps to reproduce so I don't have to dig for them.