Page MenuHomePhabricator

Various XSS issues in SocialProfile's TopUsers special pages and the <topusers> tag
Closed, ResolvedPublic

Description

Well, at least the <topusers> tag is disabled by default. The special pages, sadly, are enabled by default and not exactly disable-able.

Proposed patch:

diff --git a/UserStats/TopFansByStat.php b/UserStats/TopFansByStat.php
index 357d697..0bdd7db 100644
--- a/UserStats/TopFansByStat.php
+++ b/UserStats/TopFansByStat.php
@@ -25,6 +25,7 @@ class TopFansByStat extends UnlistedSpecialPage {
 		global $wgMemc;
 		global $wgUserStatsTrackWeekly, $wgUserStatsTrackMonthly;
 
+		$linkRenderer = $this->getLinkRenderer();
 		$lang = $this->getLanguage();
 		$out = $this->getOutput();
 		$request = $this->getRequest();
@@ -138,7 +139,6 @@ class TopFansByStat extends UnlistedSpecialPage {
 			$output .= '<h1 class="top-title">' .
 				$this->msg( 'top-fans-by-category-nav-header' )->plain() . '</h1>';
 
-			$linkRenderer = $this->getLinkRenderer();
 			$lines = explode( "\n", $message->text() );
 			foreach ( $lines as $line ) {
 				if ( strpos( $line, '*' ) !== 0 ) {
@@ -194,8 +194,11 @@ class TopFansByStat extends UnlistedSpecialPage {
 				<span class="top-fan-num">' . $x . '.</span>
 				<span class="top-fan">' .
 					$commentIcon .
-					'<a href="' . htmlspecialchars( $user_title->getFullURL() ) . '">' . $user_name . '</a>
-				</span>
+					$linkRenderer->makeLink(
+						$user_title,
+						$user_name
+					) .
+				'</span>
 				<span class="top-fan-points"><b>' . $statistics_row . '</b> ' . $lowercase_statistics_name . '</span>
 				<div class="visualClear"></div>
 			</div>';
diff --git a/UserStats/TopFansRecent.php b/UserStats/TopFansRecent.php
index 55db980..2d202c3 100644
--- a/UserStats/TopFansRecent.php
+++ b/UserStats/TopFansRecent.php
@@ -17,6 +17,7 @@ class TopFansRecent extends UnlistedSpecialPage {
 	public function execute( $par ) {
 		global $wgMemc;
 
+		$linkRenderer = $this->getLinkRenderer();
 		$out = $this->getOutput();
 		$request = $this->getRequest();
 		$user = $this->getUser();
@@ -130,7 +131,6 @@ class TopFansRecent extends UnlistedSpecialPage {
 			$output .= '<h1 class="top-title">' .
 				$this->msg( 'top-fans-by-category-nav-header' )->plain() . '</h1>';
 
-			$linkRenderer = $this->getLinkRenderer();
 			$lines = explode( "\n", $message->text() );
 			foreach ( $lines as $line ) {
 				if ( strpos( $line, '*' ) !== 0 ) {
@@ -173,8 +173,11 @@ class TopFansRecent extends UnlistedSpecialPage {
 				<span class="top-fan-num">' . $x . '.</span>
 				<span class="top-fan">' .
 					$avatarImage .
-					'<a href="' . htmlspecialchars( $user_title->getFullURL() ) . '" >' . $user['user_name'] . '</a>
-				</span>';
+					$linkRenderer->makeLink(
+						$user_title,
+						$user['user_name']
+					) .
+				'</span>';
 
 			$output .= '<span class="top-fan-points">' .
 				$this->msg( 'top-fans-points' )->numParams( $user['points'] )->parse() . '</span>';
diff --git a/UserStats/TopUsers.php b/UserStats/TopUsers.php
index d47be62..4bfa373 100644
--- a/UserStats/TopUsers.php
+++ b/UserStats/TopUsers.php
@@ -17,6 +17,7 @@ class TopUsersPoints extends SpecialPage {
 	public function execute( $par ) {
 		global $wgMemc, $wgUserStatsTrackWeekly, $wgUserStatsTrackMonthly, $wgUserLevels;
 
+		$linkRenderer = $this->getLinkRenderer();
 		$out = $this->getOutput();
 
 		// Load CSS
@@ -109,7 +110,6 @@ class TopUsersPoints extends SpecialPage {
 			$output .= '<h1 style="margin-top:15px !important;">' .
 				$this->msg( 'top-fans-by-category-nav-header' )->plain() . '</h1>';
 
-			$linkRenderer = $this->getLinkRenderer();
 			$lines = explode( "\n", $byCategoryMessage->text() );
 			foreach ( $lines as $line ) {
 				if ( strpos( $line, '*' ) !== 0 ) {
@@ -160,12 +160,12 @@ class TopUsersPoints extends SpecialPage {
 				$last_level = $user_level->getLevelName();
 			}
 
+			$userLink = $linkRenderer->makeLink( $user_title, $user['user_name'] );
 			$output .= "<div class=\"top-fan-row\">
 				<span class=\"top-fan-num\">{$x}.</span>
 				<span class=\"top-fan\">
-					{$commentIcon} <a href='" . htmlspecialchars( $user_title->getFullURL() ) . "'>" .
-						$user['user_name'] . '</a>
-				</span>';
+					{$commentIcon} {$userLink}
+				</span>";
 
 			$output .= '<span class="top-fan-points">' .
 				$this->msg( 'top-fans-points' )->numParams( $user['points'] )->parse() . '</span>';
diff --git a/UserStats/TopUsersTag.php b/UserStats/TopUsersTag.php
index 41bec46..a7a8cdc 100644
--- a/UserStats/TopUsersTag.php
+++ b/UserStats/TopUsersTag.php
@@ -56,14 +56,20 @@ function getTopUsersForTag( $input, $args, $parser ) {
 	$x = 1;
 	$topfans = '';
 
-	foreach( $fans as $fan ) {
+	$linkRenderer = MediaWiki\MediaWikiServices::getInstance()->getLinkRenderer();
+	foreach ( $fans as $fan ) {
 		$avatar = new wAvatar( $fan['user_id'], 'm' );
 		$user = Title::makeTitle( NS_USER, $fan['user_name'] );
 
+		$userLink = $linkRenderer->makeLink(
+			$user,
+			$fan['user_name']
+		);
+		$safeUserURL = htmlspecialchars( $user->getFullURL() );
 		$topfans .= "<div class=\"top-fan\">
 				<span class=\"top-fan-number\">{$x}.</span>
-				<a href=\"{$user->getFullURL()}\">{$avatar->getAvatarURL()}</a>
-				<span class=\"top-fans-user\"><a href=\"{$user->getFullURL()}\">{$fan['user_name']}</a></span>
+				<a href=\"{$safeUserURL}\">{$avatar->getAvatarURL()}</a>
+				<span class=\"top-fans-user\">{$userLink}</span>
 				<span class=\"top-fans-points\">" .
 				wfMessage( 'top-fans-points-tag' )->numParams( $fan['points'] )->parse() . '</span>
 			</div>';

cc @lcawte @SamanthaNguyen

Event Timeline

This patch looks like it fixes the issue correctly

Bawolff assigned this task to ashley.
Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".

Change 390697 merged by jenkins-bot:
[mediawiki/extensions/SocialProfile@master] [SECURITY] Fix various XSS issues in TopUsers special pages and the <topusers> tag

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