Intent to unship: Visibility of window.content to untrusted code

[Boris Zbarsky]

Bug: https://bugzilla.mozilla.org/show_bug.cgi?id=864845

window.content is a Gecko-specific thing that basically acts like
window.top in untrusted code. In chrome it returns the currently
selected tab, effectively.

I would like to unship window.content for 57; no one else implements it.

-Boris

[Emilio Cobos Álvarez]

Just for the record, since I got curious and I saw no mention in the
intent email:

I've noticed that this may be used pretty easily for UA detection. So
far [1] is the only remotely related thing I've found from a search on
Google and GitHub (outside of the firefox codebase ofc).

I suspect keeping it exposed may cause more compat issues than removing
it, and given finding _something_ was super hard I suspect this is
pretty safe to remove, but if someone wants to take a closer look,
that'd also be great, I guess.

It'd have been great to have a counter on how many times the property is
accessed from a content doc or something, but I guess it may not be
totally representative, I've seen too much code iterating over the
window properties... :P

Anyway, great to remove another non-standard feature from content
documents :)

-- Emilio

[1]: http://forums.mozillazine.org/viewtopic.php?f=25&t=232754

> _______________________________________________
> dev-platform mailing list
> dev-pl...@lists.mozilla.org
> https://lists.mozilla.org/listinfo/dev-platform

[Kohei Yoshino]

A similar story: `window.controllers` was removed with Firefox 29 but added back to Firefox 30 because it had been widely used for UA detection. `window.content` might cause the same compatibility issue, but anyway, it's difficult to guess the impact from GitHub search results...

https://www.fxsitecompat.com/en-CA/docs/2014/window-content-controllers-pkcs11-and-loadstatus-have-been-removed/

-Kohei

On 2017-09-12 5:04 PM, Emilio Cobos Álvarez wrote:
> Just for the record, since I got curious and I saw no mention in the
> intent email:
>
> I've noticed that this may be used pretty easily for UA detection. So
> far [1] is the only remotely related thing I've found from a search on
> Google and GitHub (outside of the firefox codebase ofc).
>
> I suspect keeping it exposed may cause more compat issues than removing
> it, and given finding _something_ was super hard I suspect this is
> pretty safe to remove, but if someone wants to take a closer look,
> that'd also be great, I guess.
>
> It'd have been great to have a counter on how many times the property is
> accessed from a content doc or something, but I guess it may not be
> totally representative, I've seen too much code iterating over the
> window properties... :P
>
> Anyway, great to remove another non-standard feature from content
> documents :)
>
> -- Emilio
>
> [1]: http://forums.mozillazine.org/viewtopic.php?f=25&t=232754
>
> On 09/12/2017 09:32 PM, Boris Zbarsky wrote:

[Boris Zbarsky]

On 9/12/17 5:04 PM, Emilio Cobos Álvarez wrote:
> I've noticed that this may be used pretty easily for UA detection.

Right, that and use in Gecko-only codepaths are the main concerns....

I considered adding a usecounter, but as you noted it would be affected
by window enumeration.

We could make the property non-enumerable and _then_ add a usecounter.
It would take a bit of machinery (e.g. support for non-enumerable webidl
attributes), but not that hard.

We could also delay the removal to after 57 to mitigate 57 risk....

-Boris

[Boris Zbarsky]

On 9/14/17 5:33 PM, Ehsan Akhgari wrote:
> I think either of these two ideas would be good, but I think unshipping
> in 57 is premature without having an understanding of how much the Web
> depends on this for UA sniffing.

OK. Do you have any suggestions on how we could gain that understanding?

We could try to detect sniffing like "if (window.content)" by doing
something like this:

1) Make the property non-enumerable.
2) Add a use counter for gets.

That would not detect sniffing like "if ('content' in window)"...

We could also just hide the property on nightly only and see whether we
get bug reports, possibly combined with the non-enumerable and
usecounter bits.

Any other ideas?

-Boris

[Kris Maglione]

Could we make it non-enumerable and add a counter to the window
binding resolve hook? That should catch the `"content" in
window` case (which I'd expect to be the common approach if this
is being used for UA sniffing).

[Boris Zbarsky]

On 9/14/17 5:47 PM, Kris Maglione wrote:
> Could we make it non-enumerable and add a counter to the window binding
> resolve hook?

We could. Note that it would also trigger on sets of "window.content"
or on "var content" and so forth, though... Not clear how useful the
resulting data would be, though it would give us an upper bound on the
number of sites doing anything at all with things named "content" on the
window.

For now I am going to turn off window.content for untrusted code on
nightly only. I filed
https://bugzilla.mozilla.org/show_bug.cgi?id=1400139 on adding telemetry
and https://bugzilla.mozilla.org/show_bug.cgi?id=1400140 on finishing up
the removal.

-Boris

[Ehsan Akhgari]

Unfortunately I don't have any bright ideas besides the ones you already
mentioned. :/