Page MenuHomePhabricator

CVE-2025-23072: XSS in Special:RefreshSpecial
Closed, ResolvedPublicSecurity

Description

I've found several XSSes in Special:RefreshSpecial while conducting a security review of the extension for a request on the Miraheze issue tracker. One is shown below:

2024-11-03_11-48.png (985×1 px, 704 KB)

I'm too lazy to list them all, so I'll just submit a patch that fixes them all.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Nice catches, thank you for this! 💯 👍

Some quick thoughts before we go ahead with getting this merged...

diff --git a/src/RefreshSpecial.php b/src/RefreshSpecial.php
index 312d649..eb2989c 100644
--- a/src/RefreshSpecial.php
+++ b/src/RefreshSpecial.php
@@ -58,7 +58,7 @@ class RefreshSpecial extends SpecialPage {
 		ini_set( 'memory_limit', '512M' );
 		set_time_limit( 240 );
 
-		$out->setPageTitle( $this->msg( 'refreshspecial-title' )->plain() );
+		$out->setPageTitle( $this->msg( 'refreshspecial-title' )->escaped() );
 
 		$cSF = new RefreshSpecialForm();
 		$cSF->setContext( $this->getContext() );

Is this actually exploitable? AIUI for the longest time setPageTitle was a nice catch-all method that happily, and safely, accepted whatever garbage you threw at it. You could pass in a string, a Message object, just about anything and it'd work as you'd expect it to. Not sure how it's in MW 1.4x but in 1.39 you can have whatever alerts and such passed to setPageTitle and they won't run.

diff --git a/src/RefreshSpecialForm.php b/src/RefreshSpecialForm.php
index ce9fd16..8879059 100644
@@ -1,5 +1,6 @@
 <?php
 
+use MediaWiki\Html\Html;
 use MediaWiki\MediaWikiServices;
 
 /**

The problem with this is that this de facto raises the minimum MediaWiki version from the stated 1.32 (sic) to 1.40. I'm okay with sorta crude backwards compatibility hacks to work around the namespace issue, but can we please keep 1.39 support in as it's the current LTS release? (If not, the MW requirement in extension.json would need to be adjusted accordingly.)

@@ -23,7 +24,9 @@ class RefreshSpecialForm extends ContextSource {
 
 		if ( $err != '' ) {
 			$out->setSubtitle( $this->msg( 'formerror' )->escaped() );
-			$out->addHTML( "<p class='error'>{$err}</p>\n" );
+			$out->addHTML( Html::element( 'p', [
+				'class' => 'error',
+			], $err ) . "\n" );
 		}
 
 		$out->addWikiMsg( 'refreshspecial-help' );

Though not immediately obvious, this should fix the case of the method being passed the refreshspecial-fail msg from RefreshSpecial.php. Nice!

--- a/src/RefreshSpecialForm.php
+++ b/src/RefreshSpecialForm.php
@@ -141,7 +144,7 @@ class RefreshSpecialForm extends ContextSource {
 			}
 
 			if ( !( isset( $options['only'] ) ) || ( $options['only'] == $queryPage->getName() ) ) {
-				$out->addHTML( "<b>$special</b>: " );
+				$out->addHTML( Html::element( 'b', [], $special ) . ': ' );
 
 				if ( $queryPage->isExpensive() ) {
 					$t1 = microtime( true );

Correct me if I'm mistaken here, but this doesn't seem to be an XSS issue; while it may look like one and your code is a definite improvement in code quality over the current code in master, $special comes basically from QueryPage::getPages() and thus isn't "user-controllable" in the sense that an on-wiki person, even an admin, could exploit it. (And if an attacker has SSH access to your server, this isn't likely your #1 concern anyway...)

However, just a few lines above this patched line, is this line:

$out->addWikiTextAsInterface( $this->msg( 'refreshspecial-no-page' )->plain() . ": $special\n" );

While the $special usage may indeed seem dodgy there, the same is true for plain() and yet it seems to be fine. How very particular.

@@ -228,7 +231,7 @@ class RefreshSpecialForm extends ContextSource {
 		$this->getOutput()->setSubtitle(
 			$this->msg( 'refreshspecial-choice',
 				$this->msg( 'refreshspecial-refreshing' )->plain()
-			)->plain()
+			)->escaped()
 		);
 		$this->refreshSpecial();

I tested this and it does work as it should. It's just confusing, but that's not on you, that's (IMO) on MW core. Regardless, a fine reminder that though often plain() is a code smell, sometimes it can be OK even if it still looks dodgy, and this is one of those cases.

For the curious: blame me for the XSS (sorry!), I introduced it in version 1.4.0 (5a1dcd157cbe6a6aecdcd0877be395e015401436) over a decade ago. Aren't rewrites just fun?

diff --git a/src/RefreshSpecial.php b/src/RefreshSpecial.php
index 312d649..eb2989c 100644
--- a/src/RefreshSpecial.php
+++ b/src/RefreshSpecial.php
@@ -58,7 +58,7 @@ class RefreshSpecial extends SpecialPage {
 		ini_set( 'memory_limit', '512M' );
 		set_time_limit( 240 );
 
-		$out->setPageTitle( $this->msg( 'refreshspecial-title' )->plain() );
+		$out->setPageTitle( $this->msg( 'refreshspecial-title' )->escaped() );
 
 		$cSF = new RefreshSpecialForm();
 		$cSF->setContext( $this->getContext() );

Is this actually exploitable? AIUI for the longest time setPageTitle was a nice catch-all method that happily, and safely, accepted whatever garbage you threw at it. You could pass in a string, a Message object, just about anything and it'd work as you'd expect it to. Not sure how it's in MW 1.4x but in 1.39 you can have whatever alerts and such passed to setPageTitle and they won't run.

It gets sanitized, which technically (appears to) stop <script>s and the like, but I wouldn't want to chance it.
https://doc.wikimedia.org/mediawiki-core/master/php/OutputPage_8php_source.html#l01221

diff --git a/src/RefreshSpecialForm.php b/src/RefreshSpecialForm.php
index ce9fd16..8879059 100644
@@ -1,5 +1,6 @@
 <?php
 
+use MediaWiki\Html\Html;
 use MediaWiki\MediaWikiServices;
 
 /**

The problem with this is that this de facto raises the minimum MediaWiki version from the stated 1.32 (sic) to 1.40. I'm okay with sorta crude backwards compatibility hacks to work around the namespace issue, but can we please keep 1.39 support in as it's the current LTS release? (If not, the MW requirement in extension.json would need to be adjusted accordingly.)

Whoops, didn't know that it was renamed from 1.32 to 1.42, oops. It appears that there's at least an Html class though: https://doc.wikimedia.org/mediawiki-core/REL1_32/php/classHtml.html

--- a/src/RefreshSpecialForm.php
+++ b/src/RefreshSpecialForm.php
@@ -141,7 +144,7 @@ class RefreshSpecialForm extends ContextSource {
 			}
 
 			if ( !( isset( $options['only'] ) ) || ( $options['only'] == $queryPage->getName() ) ) {
-				$out->addHTML( "<b>$special</b>: " );
+				$out->addHTML( Html::element( 'b', [], $special ) . ': ' );
 
 				if ( $queryPage->isExpensive() ) {
 					$t1 = microtime( true );

Correct me if I'm mistaken here, but this doesn't seem to be an XSS issue; while it may look like one and your code is a definite improvement in code quality over the current code in master, $special comes basically from QueryPage::getPages() and thus isn't "user-controllable" in the sense that an on-wiki person, even an admin, could exploit it. (And if an attacker has SSH access to your server, this isn't likely your #1 concern anyway...)

Yeah, I couldn't immediately think of a way to exploit it, but it's a risk, so why not eliminate it? :p
(Also, mediawikiwiki says to escape everything: https://www.mediawiki.org/wiki/XSS#Stopping_Cross-site_scripting)

However, just a few lines above this patched line, is this line:

$out->addWikiTextAsInterface( $this->msg( 'refreshspecial-no-page' )->plain() . ": $special\n" );

While the $special usage may indeed seem dodgy there, the same is true for plain() and yet it seems to be fine. How very particular.

I assume that the input is treated as wikitext and not HTML as per the function name and https://doc.wikimedia.org/mediawiki-core/REL1_32/php/classOutputPage.html#ae442f4b110eb8e56921459881a8e81c9

@@ -228,7 +231,7 @@ class RefreshSpecialForm extends ContextSource {
 		$this->getOutput()->setSubtitle(
 			$this->msg( 'refreshspecial-choice',
 				$this->msg( 'refreshspecial-refreshing' )->plain()
-			)->plain()
+			)->escaped()
 		);
 		$this->refreshSpecial();

I tested this and it does work as it should. It's just confusing, but that's not on you, that's (IMO) on MW core. Regardless, a fine reminder that though often plain() is a code smell, sometimes it can be OK even if it still looks dodgy, and this is one of those cases.

Gotta check docs :p
https://doc.wikimedia.org/mediawiki-core/REL1_32/php/classOutputPage.html#af0773075842766e0e288dc76f467219e

For the curious: blame me for the XSS (sorry!), I introduced it in version 1.4.0 (5a1dcd157cbe6a6aecdcd0877be395e015401436) over a decade ago. Aren't rewrites just fun?

Heh, I'm just glad that this time it's "only" 11 XSSes, not 18 like I've found in another extension.


Replaced MediaWiki\Html\Html with \Html

Since this isn't Wikimedia-deployed, I think we'd recommend just pushing this through gerrit for review (assuming it's patched at Miraheze) and then we can include it with the next supplemental security release.

Since this isn't Wikimedia-deployed, I think we'd recommend just pushing this through gerrit for review (assuming it's patched at Miraheze) and then we can include it with the next supplemental security release.

Pending security review from us (not yet deployed) so happy for gerrit review

mmartorana changed the visibility from "Custom Policy" to "Public (No Login Required)".Tue, Jan 14, 6:24 PM
mmartorana changed the edit policy from "Custom Policy" to "All Users".
mmartorana renamed this task from XSS in Special:RefreshSpecial to CVE-2025-23072: XSS in Special:RefreshSpecial.Tue, Jan 14, 7:18 PM