On Special:Contributions, the NS filter uses unescaped messages as keys in the 'option' key for an HTMLForm specifier (source):
$fields['nsFilters'] = [ // ... 'options' => [ $this->msg( 'invert' )->text() => 'nsInvert', $this->msg( 'namespace_association' )->text() => 'associated', ],
This is vulnerable to a mild XSS in case one of those messages is changed to include raw HTML.
The issue was spotted by taint-check.
| Status | Subtype | Assigned | Task | ||
|---|---|---|---|---|---|
| Resolved | Reedy | T256334 Release MediaWiki 1.31.9/1.34.3/1.35.0 | |||
| Resolved | Reedy | T256335 Tracking bug for MediaWiki 1.31.9/1.34.3/1.35.0 | |||
| Open | None | T2212 Some MediaWiki: messages not safe in HTML (tracking) | |||
| Resolved | Security | sbassett | T255918 Unescaped message used in HTML on Special:Contributions (CVE-2020-25812) |
Should probably security-deploy sometime soon as opposed to gerrit, just to be safe. Special page tests pass but I'm not sure if it might upset anything in production.
From 376cfd4aa749d337067982353ab334a139cf3b8e Mon Sep 17 00:00:00 2001 From: sbassett <sbassett@wikimedia.org> Date: Mon, 22 Jun 2020 16:16:15 -0500 Subject: [PATCH] Escape messages used as keys on Special:Contributions Use escaped() instead of text() for message options keys Bug: T255918 --- includes/specials/SpecialContributions.php | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/includes/specials/SpecialContributions.php b/includes/specials/SpecialContributions.php index 1a6d04f30d..eeee786123 100644 --- a/includes/specials/SpecialContributions.php +++ b/includes/specials/SpecialContributions.php @@ -590,8 +590,8 @@ class SpecialContributions extends IncludableSpecialPage { // See resources/src/mediawiki.special.recentchanges.js 'infusable' => true, 'options' => [ - $this->msg( 'invert' )->text() => 'nsInvert', - $this->msg( 'namespace_association' )->text() => 'associated', + $this->msg( 'invert' )->escaped() => 'nsInvert', + $this->msg( 'namespace_association' )->escaped() => 'associated', ], 'default' => $nsFilters, 'section' => 'contribs-top', -- 2.22.0
Let's try and see if something explodes? :-)
Jokes aside, I tried grepping those messages on codesearch and they're always used with ->text(), so they're not meant to contain any HTML, so nothing should break.
As for the patch, there's a cleaner alternative: using options-messages instead of options (see example here), as it already escapes stuff. I thought it would only work for values, but in fact it works for keys.
On a related note, LogEventsList has the same problem, see here:
private function getFiltersDesc( $filter ) { foreach ( $filter as $type => $val ) { // ... $options[ $message->text() ] = $type; // Line 214 } return [ // ... 'options' => $options, ]; }
Shall I open a new task for that, or do you want to include this fix in the same patch? For LogEventsList, options-messages cannot be used, so the fix is s/text/escaped/ at line 214.
Sounds good. Here's an updated version of the patch, which I can try to deploy during next week's (2020-06-29) security window:
From 624dc78883a117963fec1aa996fb374b36441323 Mon Sep 17 00:00:00 2001 From: sbassett <sbassett@wikimedia.org> Date: Mon, 22 Jun 2020 16:16:15 -0500 Subject: [PATCH] Escape messages used as keys on Special:Contributions Use options-messages instead of text() for message-based options keys Bug: T255918 --- includes/specials/SpecialContributions.php | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/includes/specials/SpecialContributions.php b/includes/specials/SpecialContributions.php index 1a6d04f30d..22da6b7c8d 100644 --- a/includes/specials/SpecialContributions.php +++ b/includes/specials/SpecialContributions.php @@ -589,9 +589,9 @@ class SpecialContributions extends IncludableSpecialPage { // `contribs-ns-filters` class allows these fields to be toggled on/off by JavaScript. // See resources/src/mediawiki.special.recentchanges.js 'infusable' => true, - 'options' => [ - $this->msg( 'invert' )->text() => 'nsInvert', - $this->msg( 'namespace_association' )->text() => 'associated', + 'options-messages' => [ + 'invert' => 'nsInvert', + 'namespace_association' => 'associated', ], 'default' => $nsFilters, 'section' => 'contribs-top', -- 2.22.0
Shall I open a new task for that, or do you want to include this fix in the same patch? For LogEventsList, options-messages cannot be used, so the fix is s/text/escaped/ at line 214.
Yeah, let's file a new bug.
Update: filed as T256171.
Looks like this isn't needed for REL1_31 - rMW2bb8515286da: Make Special:Contributions use OOUI added it, which is only in REL1_34