Add @-moz-document to the CSS at-rule allowlist

[westonruter]

A few WordPress themes I've been evaluating for AMP compatibility are making use of the experimental @document CSS at-rule (more specifically, @-moz-document).

Specifically, there are 69 instances 275 instances I've found across ~4,000 themes.

Every instance I've seen is used as a browser sniffing technique, for example:

@-moz-document url-prefix() {
    /* Firefox-specific rules here! */
}

Nevertheless, the MDN docs notes that this at-rule has been disabled now in stylesheets coming from webpages:

Note: There is a -moz-prefixed version of this property — @-moz-document. This has been limited to use only in user and UA sheets in Firefox 59 in Nightly and Beta — an experiment designed to mitigate potential CSS injection attacks (bug 1035091).

This being the case, perhaps this answers the question and support should not be added. Since AMP pages are never user or UA sheets, the rule would never be used by any browser yet.

[westonruter]

It turns out that the newest version of the Twenty Nineteen theme in WordPress core now makes use of @-moz-document to fix a Firefox-specific bug. This now being caught by the AMP plugin: ampproject/amp-wp#5302.

Perhaps AMP documents could allow the at-rule with no value supplied in the url-prefix(), for example:

@-moz-document url-prefix() {
   /* ... */
}

This is apparently what Firefox parses, and it ignores the value provided in the url-prefix() as I understand from bug 1446470.

As I understand, Firefox tried removing @-moz-document entirely to limit abuse, but they had to restore the parsing of it to not break a lot of Firefox-specific style rules.

[westonruter]

cc @ampproject/wg-caching

[westonruter]

Incidentally, @-moz-document was one of the most common validation errors caught by the AMP plugin when I analyzed all ~4,000 themes on WordPress.org. There are currently 275 instances.

[westonruter]

If @-moz-document were allowed it would facilitate migration to AMP.

[Gregable]

@dvoytenko @molnarg for thoughts on the css rule changes.

[dvoytenko]

I have some concerns. An AMP page can be opened from a direct origin or from a CDN. Either url-prefix should list them all, or it'd only apply in some cases but not others. This could lead to its own set of bugs. Overall we usually try to reduce any significant delta between all environments.

[westonruter]

@dvoytenko The predominant usage is for url-prefix() to be empty. As I understand from bug 1446470, this is now the only thing that Firefox parses anyway.

This can be confirmed by looking at this Pen in Firefox: https://codepen.io/westonruter/pen/eYZGOLm?editors=1100

The usage of an empty url-prefix() is referred to in this Stack Overflow answer as being just a CSS hack to target Firefox:

The url-prefix rule applies the contained style rules to any page whose URL starts with it. When used with no URL argument like @-moz-document url-prefix() it applies to ALL pages. That's effectively a CSS hack used to only target Gecko (Mozilla Firefox). All other browsers will ignore the styles.

So an empty value is the only one that Firefox now recognizes, and it's the only one that AMP should allow to support Firefox-specific CSS hacks. Even if AMP allowed a non-empty url-prefix() it wouldn't matter because Firefox ignores all non-empty values.

Almost all usages of url-prefix() in WordPress.org themes is with this empty value.

[dvoytenko]

@westonruter Ok. This sounds a bit better, but still some risks to consider if this rule can be abused by overtargetting to FF and leaving some features incomplete in other browsers. Do you have an example of a bug that you're fixing in FF this way?

[westonruter]

@dvoytenko Yes, this Firefox hack is being used to fix a rendering issue for drop caps in the Twenty Nineteen theme (actively installed on 600K+ websites) in WordPress core. Notice the difference in the drop cap's margins in non-AMP (expected) and AMP (unexpected) where in the latter the @-moz-document rule got stripped by the AMP plugin to make the page valid AMP:

See WP core commit and original WP core ticket (originally WordPress/twentynineteen#749) for more screenshots.

[dvoytenko]

@westonruter I see. This is certainly an annoying bug. Do you btw have a Firefox bug number?

[westonruter]

For unexpected margins being set on on ::first-letter? No. I didn't find anything from searching Bugzilla either.

Nevertheless, the issue is not limited to workarounds for ::first-letter. Some other examples:

@-moz-document url-prefix() {
    .control-section.open .customize-control:last-child {
        margin-bottom: 20px;
    }
}

@-moz-document url-prefix() {
    .panel-row-style {
          background-position: center !important;
    }
}

@-moz-document url-prefix() {
  #option-tree-options-layouts-form .select-wrapper {
    left: 250px;
  }
}

[kristoferbaxter]

There appears to be some confusion here, I see plenty of issues around deprecating and removing this functionality... but it does appear to work still.

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

My hunch is we should allow it, since it's an existing web compat issue.

[dvoytenko]

Should we then allowlist with only empty url-prefix for now?

[westonruter]

Yes, that would prevent it from being used in old Firefox installs for “exfiltration in CSS-injection attacks”.

[molnarg]

No concern from security PoV if we only allow empty value.

[Gregable]

Worth noting that the Validator doesn't currently have a mechanism to evaluate the url-prefix() function part of the @ rule. We would need to implement that in order to ensure an empty value. Each @ rule we add typically requires some custom code to handle it's unique parsing semantics beyond the basic CSS tokenization.

Here are the options as I see them:

1. Allow @-moz-document in AMP format (I doubt email would allow this anyway due to the browser sniffing capability) without validation on the url-prefix bit. This is a small, few-line change.
2. Add validator support for understanding and validating the remainder of the @ rule parse syntax and validate that we allow only with empty url-prefix as suggested. This is a larger change, requiring more time allocated, but doable.
3. Concluding that neither of the above options is acceptable. The first because of the security implications, and the latter because the impact is too small to invest the effort. Do nothing.

Thoughts from this group on the three options?

[westonruter]

1. Allow @-moz-document in AMP format (I doubt email would allow this anyway due to the browser sniffing capability) without validation on the url-prefix bit. This is a small, few-line change.

I understand from Bug 1035091 that Mozilla removed support for @-moz-document entirely in Firefox 59, and then via Bug 1446470 it re-added support but only recognizes url-prefix() with an empty value. So the only concern would be for users running Firefox older than v59. I'm not sure where to find detailed browser stats broken down by version, but my assumption is that there are exceedingly small number of users still running such older versions since it has auto-updates. Firefox 59 came out in March 2018—over 2 years ago.

2. Add validator support for understanding and validating the remainder of the @ rule parse syntax and validate that we allow only with empty url-prefix as suggested. This is a larger change, requiring more time allocated, but doable.

If not 1, then this is my preference.

[kristoferbaxter]

I'd prefer option 2 for completeness, and to eliminate confusion around the functionality.

[molnarg]

I'd prefer option 2 (or 3), just so that we don't depend on

• Firefox to not extend this feature once again to support more than empty values
• some other browser's decision to interpret moz- prefixes for compatibility's sake

These are both quite unlikely but also hard for us to notice.

[westonruter]

I found an alternate CSS hack for Firefox, to use @supports (-moz-appearance:meterbar) {…} instead of @-moz-document url-prefix() {…}. This being the case, I've opened ampproject/amp-wp#5530 to automatically do that replacement when the WP AMP plugin parses a stylesheet.

So this issue is no longer a blocker for the AMP plugin.

[Gregable]

I'd like to revisit this and recommend option 1 from #26406 (comment). In particular, I recommend supporting @-moz-document with any value for the "conditional group rules" section of which url-prefix() is an example.

To date, AMP validation doesn't validate this part of CSS with the exception of verifying it parses correctly. There are also rules for @media conditions in AMPEMAIL. We do not validate the conditional group rule for @font-face, @keyframes, @page, or @supports.

As far as I can tell, the only conditional group rule usable with @-moz-document is url-prefix(), and even if a value were set that a useragent respected, ex: url-prefix(https://site.example), I don't see how that would result in an actual issue with AMP.

Am I missing a specific concern?

[dvoytenko]

My main concern was that url-prefix() is essentially an if that would allow different UIs on cache vs origin. While this could be a useful feature, it also has some possibility for abuse.

[westonruter]

That usage hasn't been supported since Firefox 59 which was released almost 3 years ago. Firefox since has only supported url-prefix() with no value, as I understand.

[dvoytenko]

Ok. I guess that makes our lives a bit easier?

[Gregable]

What kind of abuse do you have in mind? If a publisher wanted to intentionally produce different UIs on cache and origin, this is already quite possible. They could respond differently based on useragent for example, giving the cache an entirely different document than user requests.

I suspect there are probably other ways to do this within a static AMP page too, either via amp javascript, mustache, or even something like a css rule that depends on an attribute the cache inserts into the html tag or something like that.

[dvoytenko]

I was thinking more about differences on different caches. But I see your point. We can relax in these circumstances.

[honeybadgerdontcare]

Fixed in #32133