Open Bug 1246578 Opened 4 years ago Updated 5 months ago

Reassess use of contentaccessible=true for marquee, pluginproblem and potentially other UA widgets

Categories

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

defect

Tracking

()

Tracking Status
firefox47 --- affected

People

(Reporter: Gijs, Unassigned)

References

(Blocks 1 open bug)

Details

(Whiteboard: [domsecurity-backlog])

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?
Blocks: 1246500
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.
(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
Whiteboard: [domsecurity-backlog]
Priority: -- → P5

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?

Flags: needinfo?(bgrinstead)

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

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.

Flags: needinfo?(bgrinstead)

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

Flags: needinfo?(bzbarsky)
Summary: Reassess use of contentaccessible=true for XBL bindings marquee and pluginproblem → Reassess use of contentaccessible=true for marquee, pluginproblem and potentially other UA widgets
See Also: → 1534581

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.

Flags: needinfo?(bzbarsky)
You need to log in before you can comment on or make changes to this bug.