Thanks for finding and reporting this issue.

Unfortunately, I'm not actively maintaining this extension.

If someone else is interested in maintaining this extension, great.

If not, maybe we should conspicuously mark this project as unmaintained and having known security issues?

Anyone know of any public wikis using this extension or other groups who would be prudent to notify?

In T368594#9931390, @GICodeWarrior wrote:

Anyone know of any public wikis using this extension or other groups who would be prudent to notify?

Per the extension page, it is being used by Miraheze, MyWikis, and WikiForge.

Cc'ing some people i think are associated with miraheze/wikiforge

In T368594#9940883, @Bawolff wrote:

Cc'ing some people i think are associated with miraheze/wikiforge

Would this be mitigated by a CSP policy?

Assuming we are talking about the data exfiltration risk, I'm not sure it could 100% fix it, but a very locked down CSP policy that restricts loadable resources could make it much more difficult to exploit

@sbassett: would you mind if this bug was shared wider with the Miraheze team to see if we can look at doing something? It's used quite widely on our farm so I'd rather not see the extension archived if there's the possibility of someone maintaining it.

In T368594#9941019, @RhinosF1 wrote:

@sbassett: would you mind if this bug was shared wider with the Miraheze team to see if we can look at doing something? It's used quite widely on our farm so I'd rather not see the extension archived if there's the possibility of someone maintaining it.

Should be fine if you want to sub a few relevant Phab users here, to discuss internally. Or even open a new, public Phab bug in regards to ext:CSS owner/maintainership.

In T368594#9941121, @sbassett wrote:

In T368594#9941019, @RhinosF1 wrote:

@sbassett: would you mind if this bug was shared wider with the Miraheze team to see if we can look at doing something? It's used quite widely on our farm so I'd rather not see the extension archived if there's the possibility of someone maintaining it.

Should be fine if you want to sub a few relevant Phab users here, to discuss internally. Or even open a new, public Phab bug in regards to ext:CSS owner/maintainership.

Thanks Scott. I've added a few of our team.

Possible Patch from @TheVoidwalker:

We are still discussing long term maintenance possibilities at https://issue-tracker.miraheze.org/T12273

In T368594#9943228, @RhinosF1 wrote:

Possible Patch from @TheVoidwalker:

0001-Use-css-sanitizer-for-css-sanitization.patch3 KBDownload

Looks reasonable to me, on it's face. I believe css-sanitizer was purpose-built for ext:TemplateStyles, but that it's design should also be general enough to allow for ad-hoc CSS sanitization elsewhere. Though @Bawolff might have more thoughts on this.

I think that fixes the issue, but you would also want to restrict urls in a similar way that the TemplateStyles does.

One of the threats that templatestyles tries to prevent is tricking users by styling stuff outside the content area (.mw-parser-output). This would still be possible in E:CSS after the patch, but probably should be considered an acceptable risk in context as something people who use this extension want.

Although its possible that one of the reasons this extension was popular is that it previously allowed things that templatestyles did not. I wonder if people will still find it useful with the enhanced santization.

Isn't it possible to not scope the selectors and to disallow all URLs? This would keep Extension:CSS' upside (styling anything outside of .mw-parser-output) whilst keeping everything the same, sans this security issue. TemplateStyles/css-sanitizer (I don't know which) is admittedly a bit stricter on stuff like CSS variables and prefers-color-scheme media queries, but we have Extension:TemplateStylesExtender.
We could also allow relative URLs if we wanted to, making this extension even more powerful than TemplateStyles.

(I haven't tested if the patch above does that—I'm currently struggling to install css-sanitizer and want to punt my computer right now)

I'm currently going through TemplateStyles's source code and basically writing what it does into the source code for E:CSS (based on Void's patch), then I came across TemplateStyles's hooks: TemplateStylesPropertySanitizer and TemplateStylesStylesheetSanitizer.

I have very little MediaWiki extension experience. Is it okay if I were to provide those hooks in E:CSS, or should I create new CSS{PropertySanitizer,StylesheetSanitizer} hooks? These hooks are used by Extension:TemplateStylesExtender, which (among other things) adds CSS variable support. I do know that at least two wikis use variables with E:CSS (IRC channel #miraheze at 2024-07-03 11:02:24), so I don't think we should leave it out.

In T368594#9961054, @BlankEclair wrote:

I have very little MediaWiki extension experience. Is it okay if I were to provide those hooks in E:CSS, or should I create new CSS{PropertySanitizer,StylesheetSanitizer} hooks?

You need new hooks.

Here's a patch based off of Void's code and Extension:TemplateStyles:

It's not fully complete yet though, mainly:

• We need to decide whether or not parsing/sanitization errors fail loudly or silently
• We need to decide how to handle error strings (do we copy them from TemplateStyles, or do we spit out the error keys from css-sanitizer to the user)
• Hook functionality is untested

Could someone answer the first two questions in the above comment?

The current implementation replaces the entire CSS content with a comment when validation fails. The comment is not particularly descriptive (e.g. /* invalid control char */ or /* insecure input */).
https://gerrit.wikimedia.org/r/plugins/gitiles/mediawiki/core/+/refs/heads/REL1_39/includes/parser/Sanitizer.php#766

I think you can keep it simple like that if you want.

Also, short error text can help avoid an amplification DOS (e.g. 1KB of malicious CSS turning into 100MB of errors).

This should have everything now. I've briefly tested hook functionality by modifying Extension:TemplateStylesExtender, and it seems to work.

Since ext:CSS isn't Wikimedia-deployed and the patch has gotten quite large, it should probably just go through gerrit at this point, to get some proper visibility and CR. Unless someone else wants to manage communications to non-Wikimedia MediaWiki operators who run ext:CSS, about patching their systems prior to any public disclosure (via gerrit, Phabricator, etc.)

Change #1074402 had a related patch set uploaded (by BlankEclair; author: BlankEclair):

[mediawiki/extensions/CSS@master] SECURITY: Use css-sanitizer for CSS sanitization

https://gerrit.wikimedia.org/r/1074402

Change #1076838 had a related patch set uploaded (by Mstyles; author: BlankEclair):

[mediawiki/extensions/CSS@REL1_42] SECURITY: Use css-sanitizer for CSS sanitization

https://gerrit.wikimedia.org/r/1076838

Change #1074402 merged by jenkins-bot:

[mediawiki/extensions/CSS@master] SECURITY: Use css-sanitizer for CSS sanitization

https://gerrit.wikimedia.org/r/1074402

Change #1076838 merged by jenkins-bot:

[mediawiki/extensions/CSS@REL1_42] SECURITY: Use css-sanitizer for CSS sanitization

https://gerrit.wikimedia.org/r/1076838