Since single and double quotes are escaped by Sanitizer::escapeHtmlAllowEntities, @import cannot be abused in this case,

I haven't tested this myself, but what about:

@import &\#x75;rl(http://example.com);

?

The css escape prevents the initial html entity decode, but then escapeHtmlAllowEntities removes it in the end to make it unsafe again.

Looking around enwikisource, it seems like most pages are using template styles, so maybe this is a legacy feature that can just be removed?

In T406088#11243629, @Bawolff wrote:

Since single and double quotes are escaped by Sanitizer::escapeHtmlAllowEntities, @import cannot be abused in this case,

I haven't tested this myself, but what about:

@import &\#x75;rl(http://example.com);

?

The css escape prevents the initial html entity decode, but then escapeHtmlAllowEntities removes it in the end to make it unsafe again.

That does indeed work.

Another PoC would be body { background: &\#x75;rl(https://http.cat/418); }

We have https://packagist.org/packages/wikimedia/css-sanitizer now which was recently used on a similar issue. That's probably a better solution than trying to use the core Sanitizer for this.

@MSantos @Jgiannelos any follow up yet?

I checked the issue and it looks like using wikimedia/css-sanitizer fixes the issue.
With a simple CSS like:

body { background: pink; }

It renders properly the background.

With the user submitted CSS from the report it sanitizes to

body {}

Here is the current WIP I have that fixes the sanitization on PageDisplayHandler::getCustomCss()

cc @cscott

Should we continue the review here since its a security patch ?

In T406088#11278152, @Jgiannelos wrote:

Here is the current WIP I have that fixes the sanitization on PageDisplayHandler::getCustomCss()

0001-Use-wikimedia-css-sanitizer-when-using-custom-CSS.patch3 KBDownload

I think we should remove the call to Sanitizer::escapeHtmlAllowEntities. This allows bypassing the CSS sanitizer by using e.g. body { content: "test";background: url(https://http.cat/418); content:""; }, which would be safe CSS on its own, since the URL token is inside a string token, but is made unsafe by decoding the entities (decoded: body { content: "test";background: url(https://http.cat/418); content:""; })
Html::inlineStyle (used by OutputPage::addInlineStyle) already takes precautions against XSS as far as I can see: It escapes < and wraps the entire thing in a CDATA section if necessary. The Sanitizer also escapes angle brackets since https://gerrit.wikimedia.org/r/c/css-sanitizer/+/358878.

Should we continue the review here since its a security patch ?

Security patches are usually reviewed in the task. The patch will be uploaded to gerrit eventually by the security team, but this is usually done shortly before the supplemental release (sometimes also earlier, but definitely only after the issue has been patched in Wikimedia production.) (cc @sbassett)

In T406088#11278152, @Jgiannelos wrote:

Here is the current WIP I have that fixes the sanitization on PageDisplayHandler::getCustomCss()

0001-Use-wikimedia-css-sanitizer-when-using-custom-CSS.patch3 KBDownload

cc @cscott

Should we continue the review here since its a security patch ?

• i dont think calling Sanitizer::escapeHtmlAllowEntities is correct here?
• the call to css sanitizer is a bit different here then how templatestyles calls it. It overrides url matching and prepends selectors. Are we sure this is safe without doing that?

It escapes < and wraps the entire thing in a CDATA section if necessary

Off topic, but just as a historical note - i think that function was written pre-html5. In modern html5 the cdata thing does nothing ( except if its inside an <svg> tag or <math> tag). Escaping the < is the important part.

In T406088#11278782, @Bawolff wrote:

It escapes < and wraps the entire thing in a CDATA section if necessary

Off topic, but just as a historical note - i think that function was written pre-html5. In modern html5 the cdata thing does nothing ( except if its inside an <svg> tag or <math> tag). Escaping the < is the important part.

Ah, I figured so while writing my comment, but assumed it was still used by some older browsers since it is still present in the function... but it does probably make sense for svg/math if it's still used there. Also for some reason, the regex checks for < even though that character is escaped beforehand...

Updated patch with simplified constructor instead of getter:

Since we use TemplateStyles in other places too, we could check if extension is loaded and use the same public static function getSanitizer() and if not create a new default instance. Not sure though if its gonna work out well because the user submitted CSS is an ad-hoc snippet that is imported in the head of the html output, not scoped like template styles.

Overall I am not sure whats the purpose of having an ad-hoc field for user submitted CSS when there is proper templatestyles support.

For example enwikisource doesn't allow this field in the config json:
https://en.wikisource.org/wiki/MediaWiki:Proofreadpage_index_data_config.json

Here are the instances that have the field enabled:

Generated using Wikimedia Global Search on 2025-10-16 11:10

FWIW, wikisource is a multilingual wiki now. The other projects are archival. What do the usages on wikisource look like?

I think there's an argument to be made for disabling the field in the config as an immediate measure, and then we can discuss the sanitizer patch in public, and turn the field back on only once that lands.

In T406088#11281408, @cscott wrote:

I think there's an argument to be made for disabling the field in the config as an immediate measure, and then we can discuss the sanitizer patch in public, and turn the field back on only once that lands.

Since there are 3rd-party installations using this extension, like Miraheze for example, I don't think this is a good idea (unless you were to modify the extension so CSS won't be added anymore even if it is enabled in the config)

I continued investigating how many pages will be affected by removing the CSS field and for this regex in all NSs that proofreadpage index is configured to use I got ~190 results:

• regex: \|Css=\s*[^\s|}]
• NSs: 252,106,102,100,104,112,110

In T406088#11282234, @Jgiannelos wrote:

I continued investigating how many pages will be affected by removing the CSS field and for this regex in all NSs that proofreadpage index is configured to use I got ~190 results:

• regex: \|Css=\s*[^\s|}]
• NSs: 252,106,102,100,104,112,110

A lot, if not most of the results are invalid CSS:

FWIW, wikisource is a multilingual wiki now. The other projects are archival.

I dont think that is true. The lang wikisources still get edits. My impression was that the multilingual project was only for languages that weren't big enough to justify a subdomain.

The more I read about it, the more it seems that TemplateStyles is the proper way to introduce custom styling eg:
https://phabricator.wikimedia.org/T215165

Overall I believe that there should be a combination of comms with communities to see if the field is supposed to be there or its just legacy.
Given the few reference of CSS in production I don't think its that problematic to disable it for security purposes but I am not sure how we can communicate that.
On top of that I am not sure how we work with 3rd parties for that. I think that overall doesn't worth the engineering time to support this field with proper sanitization.

cc @cscott

I tried to reuse what TemplateStyles does, but it gets too complicated which I am not sure if it worths the effort or if its towards the right direction.
The main problem is that the CSS field is applying a global CSS stylesheet where TemplateStyles restricts the css to specific classes (eg. defaults to CSS that is scoped under .mw-parser-output) and allows sanitized styles only under this class.
That means that in order to re-use template styles sanitizatioin we need to somehow enforce some rules to the already submitted CSS which from what I can sample from global search wont work.

My suggestion now that I understand better whats happening is a combination of what I mentioned before:

• Check if the feature is needed, if not we already have better ways to introduce custom css (TemplateStyles)
• Disable the field for security reasons since there is not gonna be massive impact (~190 index pages are defined in all prod, which translates to ~190 sources and their pages)
• (easy workaround) Use some default generic sanitizing like I did in the shared patches.

In T406088#11284545, @Jgiannelos wrote:

My suggestion now that I understand better whats happening is a combination of what I mentioned before:

• Check if the feature is needed, if not we already have better ways to introduce custom css (TemplateStyles)
• Disable the field for security reasons since there is not gonna be massive impact (~190 index pages are defined in all prod, which translates to ~190 sources and their pages)
• (easy workaround) Use some default generic sanitizing like I did in the shared patches.

If we go this route, we might want to talk to @sgrabarczuk et al to determine the best way of introducing this and communicating it to affected users.

Just checking in after being out for quite some time. cc @SLopes-WMF

• Is there consensus on disabling the CSS field in prod?
  • My understanding is yes
• Is there anything else we need from Content Transform Team side regarding the communications on deprecating the field?
• What are the next steps?

In T406088#11280691, @Jgiannelos wrote:

Since we use TemplateStyles in other places too, we could check if extension is loaded and use the same public static function getSanitizer() and if not create a new default instance. Not sure though if its gonna work out well because the user submitted CSS is an ad-hoc snippet that is imported in the head of the html output, not scoped like template styles.

Overall I am not sure whats the purpose of having an ad-hoc field for user submitted CSS when there is proper templatestyles support.

For example enwikisource doesn't allow this field in the config json:
https://en.wikisource.org/wiki/MediaWiki:Proofreadpage_index_data_config.json

Here are the instances that have the field enabled:

Generated using Wikimedia Global Search on 2025-10-16 11:10

Wiki
ban.wikisource
lij.wikisource
mad.wikisource
sah.wikisource
wikisource
tl.wikisource
yi.wikisource

This is incorrect, the actual list of wikis affected is:

Generated using Wikimedia Global Search on 2025-12-08 17:10

The only thing that ProofreadPage checks for is the presence of the "css" field in the data config file.

The migration path here is to move the CSS from the css field (or Css as it seems it often is called) of an Index page to the /styles.css subpage isn't it? And for 3rd party wikis to make sure TemplateStyles is installed.

If we search all Index namespaces (100,102,104,106,108,110,120,252) for |css= followed by anything that's likely to start a CSS string, that should find a bunch of these shouldn't it? i.e. \|[cC][sS][sS]=[a-zA-Z.#] gives these results (with a bunch of false positives because it's catching namespaces that aren't Index).

Oh sorry, ignore me I see that's already been figured out above!

In T406088#11443045, @Samwilson wrote:

The migration path here is to move the CSS from the css field (or Css as it seems it often is called) of an Index page to the /styles.css subpage isn't it? And for 3rd party wikis to make sure TemplateStyles is installed.

@Samwilson, do you know what's up with pages with invalid CSS on French Wikisource?

Relatedly, based on looking at the code https://wikisource.org/wiki/Index:Labi_1996.djvu is the only one which is a) valid CSS and b) cannot be migrated to TemplateStyles (it loads a Commons image as a background URL)

what's up with pages with invalid CSS on French Wikisource?

Do you mean Italian? With the teatro1 etc.? Those look like they're meant to be passed as the first parameter of {{TemplateStyle}} which has subpages for each of those named stylesheets. It doesn't look like MediaWiki:Proofreadpage index template actually uses the Css parameter though (or maybe there's something else that passes it on, I'm not sure).

Relatedly, based on looking at the code https://wikisource.org/wiki/Index:Labi_1996.djvu is the only one which is a) valid CSS and b) cannot be migrated to TemplateStyles (it loads a Commons image as a background URL)

It looks like the .dotted and .opaque classes are only used in Module:Content, which is used on a few Index pages — where that style doesn't load anyway.

This is another odd one, resulting in the non-functional <style>{{หรก|1=4|น={{{pagenum}}}|ล=๑๓๖|ต=๕๐ ก|ว=๑๖ เมษายน ๒๕๖๒}}</style>

Here's one that's using @import url("/wiki/中華人民共和國國務院公報/styles.css"); but that's outputting <style>/* insecure input */</style>

@Samwilson @Soda I wanted to revisit this conversation so that we can decide next steps. From the comments it does look like the CSS migration is still possible. If it's not possible, what are our other options to address this vulnerability?

I think there are still a few to be migrated to /styles.css, and a few that are invalid and for which nothing need be done.

Then, the adding of the wayward <style> element on Page pages can be removed. We could probably leave the CSS field in place in the Index page form for a while after that, because once we remove it it isn't very easy for editors to view its contents.

In T406088#11498249, @Samwilson wrote:

I think there are still a few to be migrated to /styles.css, and a few that are invalid and for which nothing need be done.

Then, the adding of the wayward <style> element on Page pages can be removed. We could probably leave the CSS field in place in the Index page form for a while after that, because once we remove it it isn't very easy for editors to view its contents.

@Samwilson can you take the lead on the migration and writing any patches for this? And keep us updated on that progress?

Sure.

I've added a section to the docs about the CSS field not being recommended.

The remaining pages to be migrated (ignoring the invalid ones such as <style>teatro7</style>) look to be:

• pl.wikisource Indeks:Dzieła dramatyczne Williama Shakespeare T. 12.djvu
• pl.wikisource Indeks:Dzieła dramatyczne Williama Shakespeare T. 2.djvu
• pl.wikisource Indeks:Dzieła dramatyczne Williama Shakespeare T. 4.djvu
• pl.wikisource Indeks:Dzieła dramatyczne Williama Shakespeare T. 5.djvu
• pl.wikisource Indeks:Dzieła dramatyczne Williama Shakespeare T. 6.djvu
• pl.wikisource Indeks:Dzieła dramatyczne Williama Shakespeare T. 7.djvu
• pl.wikisource Indeks:Dzieła dramatyczne Williama Shakespeare T. 8.djvu
• pl.wikisource Indeks:Ernest Renan - Żywot Jezusa.djvu
• pl.wikisource Indeks:H. Poincare-Wartość nauki.djvu
• pl.wikisource Indeks:PL Dumas - Sylwandira.djvu
• pl.wikisource Indeks:Podróże Gulliwera
• pl.wikisource Indeks:Potop (Sienkiewicz, wyd. 1888)
• pl.wikisource Indeks:Stanisław Goldman - Słownik Dux-Liliput.djvu
• th.wikisource ดัชนี:หลักภาษาไทย อักขรวิธี วจีวิภาค วากยสัมพันธ์ ฉันทลักษณะ.pdf
• wikisource Index:Labi 1996.djvu
• zh.wikisource Index:1958年消除就业和职业歧视公约.djvu
• zh.wikisource Index:State Council Gazette - 1990 - Issue 09.pdf
• zh.wikisource Index:State Council Gazette - 1990 - Issue 25.pdf
• zh.wikisource Index:中华人民共和国全国人民代表大会常务委员会公报2018年特刊.pdf
• zh.wikisource Index:中华人民共和国全国人民代表大会常务委员会公报2020年特刊.pdf

For this number, I don't think it's worth adding a tracking category. Unless the above global-search regex is wrong, of course, and isn't catching everything.

Here's a patch for removing the custom CSS output:

In T406088#11506153, @Samwilson wrote:

Here's a patch for removing the custom CSS output:

0003-T406088.patch3 KBDownload

+1 from me, I'll test it out locally a bit just in case.

I think the one place I differ a bit is I think we should ask folks/communities to remove the CSS field (either through an interface admin action on most wikis beside pl and zh or with some kind of MassMessage).

In T406088#11506153, @Samwilson wrote:

Here's a patch for removing the custom CSS output:

0003-T406088.patch3 KBDownload

Thank you so much! I was at an offsite last week. I can deploy this on Thursday if @Soda approves.

In T406088#11506153, @Samwilson wrote:

Here's a patch for removing the custom CSS output:

0003-T406088.patch3 KBDownload

Deployed

@Mstyles Should I create the change on Gerrit now?

In T406088#11507283, @Soda wrote:

I think the one place I differ a bit is I think we should ask folks/communities to remove the CSS field (either through an interface admin action on most wikis beside pl and zh or with some kind of MassMessage).

That's a good idea. Now the field doesn't do anything, Wikisources can remove it whenever they're happy that the content has been migrated or is no longer applicable.

@Samwilson we will wait until this is publicly announced in the supplemental release before pushing to Gerrit.

In T406088#11547582, @Samwilson wrote:

@Mstyles Should I create the change on Gerrit now?

As Maryum noted, we'd typically wait until the next supplemental security release (due out around the end of March 2026) to merge, backport and disclose this issue. But if the codebase is fairly volatile and we'll have to provide rebased security patches here every week or two, then it can be merged to master and backported early.

Okay great, thanks. PRP doesn't see that many changes, so it's fine to wait I think.

General feedback, can we/do we have a timetable of (upcoming) security fix deployment windows and supplemental releases on wikitech? I keep missing these and finding out about them through phab comments :(

@Soda releases are at the end of every quarter. I don't think we have a formal calendar. But there are the tracking tickets (T411394 and T411384), and those should help.

I can't see those tasks. I assume they're security-restricted :( A public/NDA-restricted calendar/tracking task of some kind would be nice tbh!

In T406088#11555329, @Soda wrote:

I can't see those tasks. I assume they're security-restricted :( A public/NDA-restricted calendar/tracking task of some kind would be nice tbh!

It's difficult to formalize an exact schedule for security releases, as they fluctuate due to changing assignees, competition with different quarterly goals, vacations/breaks/offistes, and other random events. We do strive to get the releases out by the end of each quarter though (March 31st, June 30th, September 30th, December 31st) some times a few days early, sometimes a few days later.

As for security deployments to Wikimedia production, those typically happen during the weekly security deployment window (ex: this week) at the same time each week. Though sometimes we also attempt to perform them after the late backport window on Thursdays. We try to avoid emergency security patch deployments at odd hours and times but those do happen from time to time.

Any objections for this patch to go through Gerrit?

Not from my end. To my understanding the on-wiki stuff is either invalid or has been moved to sanitized-css subpages.

Yep, I also think it's good to go.

Change #1266266 had a related patch set uploaded (by Reedy; author: Samwilson):

[mediawiki/extensions/ProofreadPage@master] SECURITY: Remove custom CSS Index page field

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

I've fixed a PHPCS issue, dropped a couple more tests that were failing because they were directly referencing removed functions..

There's a test failure in one remaining test, if someone could please have a look:

14:59:46 1) ProofreadPage\Page\PageDisplayHandlerTest::testGetIndexFieldsForJSForCSSWithInsecureInput
14:59:46 Failed asserting that two strings are identical.
14:59:46 --- Expected
14:59:46 +++ Actual
14:59:46 @@ @@
14:59:46 -'/* insecure input */'
14:59:46 +'background: url('/my-bad-url.jpg');'
14:59:46 
14:59:46 /workspace/src/extensions/ProofreadPage/tests/phpunit/Page/PageDisplayHandlerTest.php:115

Does the expected output just want updating to use what's now being output?

@Reedy I would just drop that test as well, it is testing for the existence of the sanitizer, which is no longer needed because the CSS field does not do anything useful anymore

Change #1266339 had a related patch set uploaded (by SomeRandomDeveloper; author: Samwilson):

[mediawiki/extensions/ProofreadPage@REL1_45] SECURITY: Remove custom CSS Index page field

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

Change #1266340 had a related patch set uploaded (by SomeRandomDeveloper; author: Samwilson):

[mediawiki/extensions/ProofreadPage@REL1_44] SECURITY: Remove custom CSS Index page field

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

Change #1266266 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@master] SECURITY: Remove custom CSS Index page field

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

Change #1266341 had a related patch set uploaded (by SomeRandomDeveloper; author: Samwilson):

[mediawiki/extensions/ProofreadPage@REL1_43] SECURITY: Remove custom CSS Index page field

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

Change #1266339 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@REL1_45] SECURITY: Remove custom CSS Index page field

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

Change #1266341 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@REL1_43] SECURITY: Remove custom CSS Index page field

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

Change #1266340 merged by jenkins-bot:

[mediawiki/extensions/ProofreadPage@REL1_44] SECURITY: Remove custom CSS Index page field

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

Is there anything else to do, or can this be closed now?

In T406088#11791137, @SomeRandomDeveloper wrote:

Is there anything else to do, or can this be closed now?

Just release T411394. The Security-Team plans to resolve and make this task public (along with the other tasks in the release) sometime this week.