Page MenuHomePhabricator

i18n XSS vulnerability in message 'readinglists-error'
Closed, ResolvedPublicSecurity

Description

Discovered with $wgUseXssLanguage.

To reproduce:

Visit Special:ReadingLists with ?uselang=x-xss

Details

Risk Rating
Low
Author Affiliation
Wikimedia Communities
Related Changes in Gerrit:

Event Timeline

Patch that should fix this:

diff --git a/src/SpecialReadingLists.php b/src/SpecialReadingLists.php
index 1e71c78..ad1cb6b 100644
--- a/src/SpecialReadingLists.php
+++ b/src/SpecialReadingLists.php
@@ -25,7 +25,7 @@ class SpecialReadingLists extends UnlistedSpecialPage {
 		$out->addModules( [ 'special.readinglist.scripts' ] );
 		$out->setPageTitleMsg( $this->msg( 'readinglists-special-title' ) );
 		$html = Html::errorBox(
-			$this->msg( 'readinglists-error' )->text(),
+			$this->msg( 'readinglists-error' )->parse(),
 			'',
 			'reading-list__errorbox'
 		);

Is this ok to upload via Gerrit?

The above patch as a file:

The above patch as a file:

Looks good to me, CR+1.

sbassett subscribed.

The above patch as a file:

CR+1. IMO, this is low-risk enough to just go through gerrit and hopefully ride next week's train.

@sbassett If I submit it via Gerrit, should I do so with the same commit message as in the patch above (including the "SECURITY:" tag and mention of XSS vulnerability), or should I be more vague?

@sbassett If I submit it via Gerrit, should I do so with the same commit message as in the patch above (including the "SECURITY:" tag and mention of XSS vulnerability), or should I be more vague?

I don't think it's required to use the SECURITY: prefix in this case. In most cases, we've considered these Message API sanitizations to be low-risk and have just done them publicly in gerrit (see T2212 and subtasks) as one would still need int-admin rights or to find some bypass of translatewiki.net to exploit them.

as one would still need int-admin rights or to find some bypass of translatewiki.net to exploit them.

I don’t think that’s quite right? You only need sysop rights to exploit these, and they effectively allow sysops to escalate to interface-admin. (Non-JS pages in the MediaWiki namespace require editinterface, which is granted to sysops; interface admins additionally have editsitejs.)

I don’t think that’s quite right? You only need sysop rights to exploit these, and they effectively allow sysops to escalate to interface-admin. (Non-JS pages in the MediaWiki namespace require editinterface, which is granted to sysops; interface admins additionally have editsitejs.)

Ah, ok. As long as it still requires elevated privileges to actually edit affected messages, then I would still consider it to be low-risk to fix most of these types of XSSes publicly.

Change #1014541 had a related patch set uploaded (by Jon Harald Søby; author: Jon Harald Søby):

[mediawiki/extensions/ReadingLists@master] Change message format for 'readinglists-error'

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

Change #1014541 merged by jenkins-bot:

[mediawiki/extensions/ReadingLists@master] Change message format for 'readinglists-error'

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

Jdlrobson-WMF subscribed.

@sbassett can this be resolved?

sbassett assigned this task to jhsoby.
sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.
sbassett removed a project: Patch-For-Review.

@sbassett can this be resolved?

Yes.

sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".Jan 31 2025, 6:16 PM
sbassett changed the edit policy from "Custom Policy" to "All Users".
sbassett changed Risk Rating from N/A to Low.