Also ccing @Soda here in case you're interested in this, as it looks like you've been working on UploadWizard a bit recently (though to be clear, I don't mean to imply any obligation on you to fix this bug!)

Thanks for the patch @Soda! Mini review:

Subject: [PATCH] Parse titleblacklist message as wikitext before showing in
 error dialog

Nitpick: the commit-message subject for security fixes should be prefixed with SECURITY: IIUC.

-	"mwe-upwiz-blacklisted-details-titleblacklist-custom-filename-text": "<p><b>Examples of good file names:</b></p><ul><li>Nodnol skyline from Nodnol City Hall - Aug 2022</li><li>1875 Meeting of Settlers at Falconer Bay, New Nodland</li><li>Pseudohedron with no vertex visible from center</li></ul><br><p><b>Examples of bad file names:</b></p><ul><li>Image01</li><li>Joe</li><li>DSC00001</li><li>Foo.svg.png</li><li>30996951316264l</li><li>PSEUDOHEDRON WITH NO VERTEX VISIBLE FROM CENTER</li></ul><p><a href='https://commons.wikimedia.org/wiki/MediaWiki:Titleblacklist-custom-filename' target='_blank'>Learn more</a></p>",
+	"mwe-upwiz-blacklisted-details-titleblacklist-custom-filename-text": "<p><b>Examples of good file names:</b></p><ul><li>Nodnol skyline from Nodnol City Hall - Aug 2022</li><li>1875 Meeting of Settlers at Falconer Bay, New Nodland</li><li>Pseudohedron with no vertex visible from center</li></ul><br><p><b>Examples of bad file names:</b></p><ul><li>Image01</li><li>Joe</li><li>DSC00001</li><li>Foo.svg.png</li><li>30996951316264l</li><li>PSEUDOHEDRON WITH NO VERTEX VISIBLE FROM CENTER</li></ul><p>[https://commons.wikimedia.org/wiki/MediaWiki:Titleblacklist-custom-filename Learn more]</p>",

Following the change from .text() to .parse() in uw.TitleDetailsWidget.js, the <br> in this system message doesn't seem to be parsed:

Functionality-wise, this patch also loses the benefit of the 'Learn more' link opening in a new tab (given that it was the <a> tag's target='_blank' attribute that previously caused this behaviour). I'm not immediately sure whether there's another way to achieve this, though.

(There's also the general advice to avoid touching i18n files in patches for security-deployment, though I'll leave that for the secteam to comment on whether it'd be an issue here or not [given they'd probably be the folks deploying it].)

It's probably also worth noting that (IIUC) the translations for mwe-upwiz-blacklisted-details-titleblacklist-custom-filename-text from translatewiki would break slightly if/when this patch gets deployed to Wikimedia production (see screenshot); given that the <a>-tags they contain would no longer be parsed. (Actually, maybe that's also a reason in favour of temporarily hard-coding the modified system message?)

Anyhow -- on the matter of the XSS, I tested the patch locally; and (at a first glance), with it applied, I no longer seem to be able to reproduce it using the steps in the task description :)

Also, FYI all, after searching through the UploadWizard code some more, I've found a couple more i18n-XSSes within the extension (with slightly different reproduction steps to this one) [0]. Please could someone let me know if they'd prefer me to combine them into this security task, or if it'd be better to open up separate security tasks for them? I'm happy to do either.

[0] They're similar but different -- in that they each have different reproduction steps, but they each relate to system messages that get passed into UploadWizard's mw.errorDialog function.

In T407157#11276713, @A_smart_kitten wrote:

Please could someone let me know if they'd prefer me to combine them into this security task, or if it'd be better to open up separate security tasks for them?
...
[0] They're similar but different -- in that they each have different reproduction steps, but they each relate to system messages that get passed into UploadWizard's mw.errorDialog function.

If it's not too many, I'd recommend adding them to the patch here since they're for the same extension and roughly the same issue (system message XSSes).

Ack; will add them to the task description when I get a chance (probably tomorrow). ty!

In T407157#11276478, @A_smart_kitten wrote:

Following the change from .text() to .parse() in uw.TitleDetailsWidget.js, the <br> in this system message doesn't seem to be parsed:

This might be partially-related to this current, still-undisclosed security issue: T251032. Basically, parse() doesn't actually parse wikitext or html in the sense of running it through parsoid or MediaWiki's parser. It parses MediaWiki messages via mw.format and then does some last-minute, basic html-escaping, the same as escaped().

In T407157#11281648, @sbassett wrote:

In T407157#11276478, @A_smart_kitten wrote:

Following the change from .text() to .parse() in uw.TitleDetailsWidget.js, the <br> in this system message doesn't seem to be parsed:

This might be partially-related to this current, still-undisclosed security issue: T251032. Basically, parse() doesn't actually parse wikitext or html in the sense of running it through parsoid or MediaWiki's parser. It parses MediaWiki messages via mw.format and then does some last-minute, basic html-escaping, the same as escaped().

That not-actually-a-parser is replaced by a real parser (but quite limited) when the 'mediawiki.jqueryMsg' module is loaded, here: https://gerrit.wikimedia.org/g/mediawiki/core/+/050bc8d455e70c3306a58fd52e4710124aea8c6f/resources/src/mediawiki.jqueryMsg/mediawiki.jqueryMsg.js#1495

I think its parser doesn't support self-closing HTML tags. I'm not sure if that's an intended limitation or a bug, it's not mentioned either way at https://www.mediawiki.org/wiki/Manual:Messages_API#Feature_support_in_JavaScript.

Since this message doesn't take any parameters, but it is otherwise quite complex, it would probably be better to parse it server-side using the normal wikitext parser. The result can be included as part of the ResourceLoader module and displayed by the client-side code. I think this could be done in a security patch, but if it seems like too big of a change, it could also happen in a non-security patch later. Here's an example from another extension:

• https://gerrit.wikimedia.org/g/mediawiki/extensions/DiscussionTools/+/ce82a1f8d062ec9c914243587674fcd3404010b3/extension.json#226
• https://gerrit.wikimedia.org/g/mediawiki/extensions/DiscussionTools/+/ce82a1f8d062ec9c914243587674fcd3404010b3/includes/ResourceLoaderData.php#85
• https://gerrit.wikimedia.org/g/mediawiki/extensions/DiscussionTools/+/ce82a1f8d062ec9c914243587674fcd3404010b3/modules/dt.ui.ReplyWidget.js#5

In T407157#11279407, @A_smart_kitten wrote:

Ack; will add them to the task description when I get a chance (probably tomorrow). ty!

Done - apologies, in the process of typing them up, I ended up finding even more that I didn't know about yesterday. I tried as best I could to not make the task description overly long, while still conveying enough reproduction information for each of them. Let me know if it'd be better to split any of them out into their own task(s); I'd be happy to do so.

In T407157#11282744, @matmarex wrote:

In T407157#11281648, @sbassett wrote:

In T407157#11276478, @A_smart_kitten wrote:

Following the change from .text() to .parse() in uw.TitleDetailsWidget.js, the <br> in this system message doesn't seem to be parsed:

This might be partially-related to this current, still-undisclosed security issue: T251032. Basically, parse() doesn't actually parse wikitext or html in the sense of running it through parsoid or MediaWiki's parser. It parses MediaWiki messages via mw.format and then does some last-minute, basic html-escaping, the same as escaped().

That not-actually-a-parser is replaced by a real parser (but quite limited) when the 'mediawiki.jqueryMsg' module is loaded, here: https://gerrit.wikimedia.org/g/mediawiki/core/+/050bc8d455e70c3306a58fd52e4710124aea8c6f/resources/src/mediawiki.jqueryMsg/mediawiki.jqueryMsg.js#1495

I think its parser doesn't support self-closing HTML tags. I'm not sure if that's an intended limitation or a bug, it's not mentioned either way at https://www.mediawiki.org/wiki/Manual:Messages_API#Feature_support_in_JavaScript.

FWIW, when attempting to mentally work my way through the relevant code here yesterday, my thoughts were that:

• the parserDefaults variable in jqueryMsg seems to get its list of allowed HTML elements from the server,
• which seems to then generate its list of allowed 'single' html elements by finding the elements in Sanitizer::getRecognizedTagData()'s htmlsingle array that aren't present in its htmlsingleonly array;
• and <br> _is_ in the htmlsingleonly array; so therefore, I guess it doesn't end up in parserDefaults' list of allowed HTML elements.

git blame on the code in Resources.php takes me back to https://gerrit.wikimedia.org/r/85153; the commit message for which suggests that the limitation is at least known about (and is intentionally coded for on the server-side).

(Also, to be clear @sbassett I know it's not your fault personally & I want to be clear that I don't mean anything ill against you, or anything like that. But I have to be honest: one of my first reactions to seeing a still-open security task that starts with T25 was something like... :( *sadface*. Again, I know it's not your own personal fault here, but I really wish more WMF resources were put into fixing security issues & fixing them quickly.)

In T407157#11282903, @A_smart_kitten wrote:

(Also, to be clear @sbassett I know it's not your fault personally & I want to be clear that I don't mean anything ill against you, or anything like that. But I have to be honest: one of my first reactions to seeing a still-open security task that starts with T25 was something like... :( *sadface*. Again, I know it's not your own personal fault here, but I really wish more WMF resources were put into fixing security issues & fixing them quickly.)

No offense taken. I don't want to derail this task too much, but I'll state that the Security-Team (now a part of Product Safety and Integrity) is tasked with many responsibilities, including reviewing anything that comes into Phab as a "security issue". We've been able to mostly stay on top of these tasks by at least triaging them during our weekly clinics, and trying to get more serious issues quickly resolved. But sometimes that doesn't happen for a number of reasons and is likely why we still have quite the security issue backlog. While many of those 1,069 tasks are probably low-risk and/or irrelevant at this point, a good portion are likely higher risk and still a concern. I would encourage you or anyone in the Community who finds this concerning to mention this during any engagement with WMF staff and leadership, be those feedback sessions, Community requests, wish list items, etc.

cc @SomeRandomDeveloper in case you're interested as you've been doing things around i18n-XSSes recently :)

Suggested patch that should cover all XSSs mentioned in the task description:

Current issues:

• The link in mwe-upwiz-blacklisted-details-titleblacklist-custom-filename-text doesn't open in a new tab anymore (I guess we could construct the link using jQuery and insert it as a parameter if that's necessary?)
• <br><br> in mwe-upwiz-license-external-invalid is not parsed
• <br> in mwe-upwiz-blacklisted-details-titleblacklist-custom-filename-text is not parsed

I haven't looked into the code yet that is causing <br> to not be parsed, but if nobody else has written a patch for that yet or is considering writing a patch, I could look into that as well.

I'm not familiar enough with the UploadWizard extension to feel comfortable giving a +1 here; but after applying the patch locally, I can no longer repro the XSSes listed in the description.
A few extra notes on the patch (F69894453), in addition to what you've already noted:

• IIUC, Bug: T407157 should be added as a footer in the commit message.
• The advice to avoid touching i18n files in patches for security-deployment also applies AFAIK, but tbh I'll let the Security-Team comment on whether that'd be a problem in terms of deployment.
• As in T407157#11276478, the translations for [...]-titleblacklist-custom-filename-text from translatewiki would break slightly if/when the patch gets deployed to WMF prod, due to containing <a> tags that would no longer be parsed. For the patch that gets deployed, I wonder if that's a reason in favour of temporarily hard-coding the (updated) English version of that message?

The link in mwe-upwiz-blacklisted-details-titleblacklist-custom-filename-text doesn't open in a new tab anymore

Re this, one worry is that - if people click the link, and then get taken to a different page in their current tab (which they're using UploadWizard in) - they might worry that their progress in the upload wizard has gone away, and that they'll have to go through the process again from the start. I'll let someone else decide whether this'd be an issue for the patch here, though.

In T407157#11345853, @A_smart_kitten wrote:

• IIUC, Bug: T407157 should be added as a footer in the commit message.
• The advice to avoid touching i18n files in patches for security-deployment also applies AFAIK, but tbh I'll let the Security-Team comment on whether that'd be a problem in terms of deployment.
• As in T407157#11276478, the translations for [...]-titleblacklist-custom-filename-text from translatewiki would break slightly if/when the patch gets deployed to WMF prod, due to containing <a> tags that would no longer be parsed. For the patch that gets deployed, I wonder if that's a reason in favour of temporarily hard-coding the (updated) English version of that message?

Updated patch which specifies the bug in the commit message (I keep forgetting this for some reason) and hardcodes the affected message in JS while not modifying any i18n files:

The link in mwe-upwiz-blacklisted-details-titleblacklist-custom-filename-text doesn't open in a new tab anymore

Re this, one worry is that - if people click the link, and then get taken to a different page in their current tab (which they're using UploadWizard in) - they might worry that their progress in the upload wizard has gone away, and that they'll have to go through the process again from the start.

Good point. I just tried this and fortunately, there is a prompt that prevents this from happening:

(I'm getting a bit off-topic here, but issues like this are why I think that we either need to improve jqueryMsg by e.g. adding support for <br> and links that open in a new tab, properly documenting how to use raw parameters, etc. so it's easier to avoid both XSSs and complicated logic, or server-side-parsed messages should be easier to use. The current state of the APIs available in JS, which require complicated workarounds for simple things like having a link open in a new tab without having to add two separate messages, is not really great.)

In T407157#11347254, @SomeRandomDeveloper wrote:

For the gerrit patch, we'd need to extract "Learn more" into a new message, build a link with this text using jQuery, and then pass it to mwe-upwiz-blacklisted-details-titleblacklist-custom-filename-text as a parameter.

I just realized that in theory, we could also go through the nodes of the parsed DOM returned by jqueryMsg and set the target of each link to _blank. However, that's a worse solution IMO.

In T407157#11347254, @SomeRandomDeveloper wrote:

The link in mwe-upwiz-blacklisted-details-titleblacklist-custom-filename-text doesn't open in a new tab anymore

Re this, one worry is that - if people click the link, and then get taken to a different page in their current tab (which they're using UploadWizard in) - they might worry that their progress in the upload wizard has gone away, and that they'll have to go through the process again from the start.

Good point. I just tried this and fortunately, there is a prompt that prevents this from happening:

Unfortunately, I don't know if that's supported/respected by all browsers :/
[edit: changed link]

I've considered multiple solutions here, such as using multiple messages and building the link using jQuery, or creating the link/message on the server, but the simplest, albeit not the cleanest, solution seems to be to add the target="_blank" attribute to all links in the parsed message using jQuery. I didn't add a check for the message key as IMO it makes sense to do this for any links displayed in the modal.
Updated patch:

I've started working on a core patch to add support for self-closing tags like <br> to jqueryMsg (I've got it working, but I still need to properly test it and do some cleaning up). I assume it's fine if this goes through the normal gerrit review process and gets a separate public task, since it's not specifically related to this vulnerability?

In T407157#11357029, @SomeRandomDeveloper wrote:

I've started working on a core patch to add support for self-closing tags like <br> to jqueryMsg (I've got it working, but I still need to properly test it and do some cleaning up). I assume it's fine if this goes through the normal gerrit review process and gets a separate public task, since it's not specifically related to this vulnerability?

Not a member of the Security-Team but I would personally think so. (Although FWIW, if you're planning to rely on that proposed core patch for the UW security patch, it'd presumably need to be backported to at least REL1_43; but I don't know whether feature-adding patches such as that are typically backported.)

In T407157#11357344, @A_smart_kitten wrote:

if you're planning to rely on that proposed core patch for the UW security patch, it'd presumably need to be backported to at least REL1_43; but I don't know whether feature-adding patches such as that are typically backported.

Ideally I would prefer backporting this if possible, since it's a rather small change code-wise and it's not a user-facing feature (it mostly gives developers more options). The alternative would be using server-side parsing in UW, but that's not as easy as in the WikiLove patch because ext.uploadWizard uses scripts and migrating to packageFiles doesn't seem feasible for a security patch. Using virtual files with scripts also seems to be possible since MW 1.41, but as far as I can see it would require a more hacky solution on the JS-side. In the end, we also still have to fix the link so it opens in a new tab, which afaik isn't possible with the PHP parser either, so we'd either have to construct the link using Html::element and two separate messages, or re-parse the HTML in JS to modify it, which is also not optimal.
Ideally, I'd also like to hear the maintainer's opinion on this (assuming that the extension has active maintainers) to figure out what solution makes more sense in the context of the codebase.

I found T295568: Support single HTML tags in mediawiki.jqueryMsg to track the core patch.

Fixed version of the patch in T407157#11356911 to use .find instead of .children so all elements are selected:

In T407157#11358704, @SomeRandomDeveloper wrote:

Fixed version of the patch in T407157#11356911 to use .find instead of .children so all elements are selected:

01-T407157-open-link-in-new-tab-2.patch7 KBDownload

Patch looks reasonable to me. We can try to get this deployed during today's window.

In T407157#11360805, @sbassett wrote:

In T407157#11358704, @SomeRandomDeveloper wrote:

Fixed version of the patch in T407157#11356911 to use .find instead of .children so all elements are selected:

01-T407157-open-link-in-new-tab-2.patch7 KBDownload

Patch looks reasonable to me. We can try to get this deployed during today's window.

The <br> tags in three messages are not parsed by jqueryMsg currently; this patch depends on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1203448

Ideally, I'd also like to hear the maintainer's opinion on this (assuming that the extension has active maintainers) to figure out what solution makes more sense in the context of the codebase.

For the record, whether or not UploadWizard has an active maintainer is unfortunately a thing in itself (T404286: Clarify the status of Structured Content / Structured Data team, T401435#11280987). It was previously the Structured Data team, but then that seems to no longer be the case now that the SD team seems to no longer exist.

In T407157#11361165, @SomeRandomDeveloper wrote:

The <br> tags in three messages are not parsed by jqueryMsg currently; this patch depends on https://gerrit.wikimedia.org/r/c/mediawiki/core/+/1203448

With https://gerrit.wikimedia.org/r/1203448 now merged, it should ride this week's train, and then we can deploy the aforementioned security patch this Thursday, 2025-11-20.

In T407157#11358704, @SomeRandomDeveloper wrote:

Fixed version of the patch in T407157#11356911 to use .find instead of .children so all elements are selected:

01-T407157-open-link-in-new-tab-2.patch7 KBDownload

Deployed

I'll look into creating a separate patch for 1.44, 1.43 and 1.39 soon. @Catrope do you think we could backport https://gerrit.wikimedia.org/r/1203448 to 1.45, so we can at least use the easier UW patch for that version?

Ah damn, @SomeRandomDeveloper I closed the subtask forgetting about your request in T407157#11358305 to backport the patch. Apologies & feel free to reopen it!

In T407157#11398519, @A_smart_kitten wrote:

Ah damn, @SomeRandomDeveloper I closed the subtask forgetting about your request in T407157#11358305 to backport the patch. Apologies & feel free to reopen it!

No worries, it wasn't a goal of the subtask to backport the change, so I would consider it resolved as well. The backport would only be useful for this task.

The jqueryMsg patch for T295568 was backported to REL1_44 and REL1_43, so we can use the same fix for all branches.

Gerrit patch for master, REL1_45 and REL1_44 (without the hardcoded message):

1.39 is going to be EOL this month, so I'm not sure whether it's worth creating a separate patch for REL1_39, assuming the supplemental won't happen before the EOL date...

Change #1224146 had a related patch set uploaded (by Mmartorana; author: SomeRandomDeveloper):

[mediawiki/extensions/UploadWizard@master] SECURITY: Fix several stored i18n XSS vulnerabilities

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

Change #1224146 abandoned by Mmartorana:

[mediawiki/extensions/UploadWizard@master] SECURITY: Fix several stored i18n XSS vulnerabilities

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

Change #1224146 restored by Mmartorana:

[mediawiki/extensions/UploadWizard@master] SECURITY: Fix several stored i18n XSS vulnerabilities

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

Change #1224711 had a related patch set uploaded (by Mmartorana; author: SomeRandomDeveloper):

[mediawiki/extensions/UploadWizard@REL1_43] SECURITY: Fix several stored i18n XSS vulnerabilities

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

Change #1224711 merged by jenkins-bot:

[mediawiki/extensions/UploadWizard@REL1_43] SECURITY: Fix several stored i18n XSS vulnerabilities

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

Change #1224146 merged by jenkins-bot:

[mediawiki/extensions/UploadWizard@master] SECURITY: Fix several stored i18n XSS vulnerabilities

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

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

[mediawiki/extensions/UploadWizard@REL1_45] SECURITY: Fix several stored i18n XSS vulnerabilities

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

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

[mediawiki/extensions/UploadWizard@REL1_44] SECURITY: Fix several stored i18n XSS vulnerabilities

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

Change #1224830 merged by jenkins-bot:

[mediawiki/extensions/UploadWizard@REL1_44] SECURITY: Fix several stored i18n XSS vulnerabilities

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

Change #1224829 merged by jenkins-bot:

[mediawiki/extensions/UploadWizard@REL1_45] SECURITY: Fix several stored i18n XSS vulnerabilities

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