From 60dbf6ba42ee3cb3a31ba8dcd696c6a5dd9f64ca Mon Sep 17 00:00:00 2001
From: Brian Wolff <bawolff+wn@gmail.com>
Date: Fri, 23 Feb 2018 21:31:28 +0000
Subject: [PATCH] SECURITY: Respect revision delete in Special:Redirect

Special:Redirect could potentially leak information about
revdel'd log entries and users (hideuser) to users who are not
supposed to see this information.

Bug: T187638
Change-Id: I16d4ec0801e8dda158ee12b8e3ebeae1878e051a
---
 includes/specials/SpecialRedirect.php | 45 ++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)

diff --git a/includes/specials/SpecialRedirect.php b/includes/specials/SpecialRedirect.php
index 3273046..0a3d5a9 100644
--- a/includes/specials/SpecialRedirect.php
+++ b/includes/specials/SpecialRedirect.php
@@ -79,6 +79,9 @@ class SpecialRedirect extends FormSpecialPage {
 		if ( $user->isAnon() ) {
 			return null;
 		}
+		if ( $user->isHidden() && !$this->getUser()->isAllowed( 'hideuser' ) ) {
+			return null;
+		}
 		$userpage = Title::makeTitle( NS_USER, $username );
 
 		return $userpage->getFullURL( '', false, PROTO_CURRENT );
@@ -179,19 +182,24 @@ class SpecialRedirect extends FormSpecialPage {
 
 		$logparams = [
 			'log_id',
+			'log_deleted',
 			'log_timestamp',
 			'log_type',
 			'log_user_text',
 		];
 
+		$user = $this->getUser();
+
 		$dbr = wfGetDB( DB_REPLICA );
 
+		// Exclude logs user does not have right to view
+		$restrictClause = LogEventsList::getExcludeClause( $dbr, 'user', $user ) ?: '1=1';
 		// Gets the nested SQL statement which
 		// returns timestamp of the log with the given log ID
 		$inner = $dbr->selectSQLText(
 			'logging',
 			[ 'log_timestamp' ],
-			[ 'log_id' => $logid ]
+			[ 'log_id' => $logid, $restrictClause ]
 		);
 
 		// Returns all fields mentioned in $logparams of the logs
@@ -199,7 +207,7 @@ class SpecialRedirect extends FormSpecialPage {
 		$logsSameTimestamps = $dbr->select(
 			'logging',
 			$logparams,
-			[ "log_timestamp = ($inner)" ]
+			[ "log_timestamp = ($inner)", $restrictClause ]
 		);
 		if ( $logsSameTimestamps->numRows() === 0 ) {
 			return null;
@@ -213,13 +221,36 @@ class SpecialRedirect extends FormSpecialPage {
 			}
 		}
 
-		array_shift( $logparams );
+		// get rid of log_id and log_deleted.
+		unset( $logparams[0] );
+		unset( $logparams[1] );
 
 		// Stores all the rows with the same values in each column
 		// as $rowMain
 		foreach ( $logparams as $cond ) {
 			$matchedRows = [];
+			if (
+				$cond === 'log_user_text' &&
+				!LogEventsList::userCan(
+					$rowMain, LogPage::DELETED_USER, $user
+				)
+			) {
+				// This field is redacted for main log entry,
+				// so treat as always matching.
+				$matchedRows = $logsSameTimestamps;
+				continue;
+			}
 			foreach ( $logsSameTimestamps as $row ) {
+				if (
+					$cond === 'log_user_text' &&
+					!LogEventsList::userCan(
+						$row, LogPage::DELETED_USER, $user
+					)
+				) {
+					// The comparision row is redacted, but the main
+					// row is not. Treat this as a non-match.
+					continue;
+				}
 				if ( $row->$cond === $rowMain->$cond ) {
 					$matchedRows[] = $row;
 				}
@@ -239,6 +270,14 @@ class SpecialRedirect extends FormSpecialPage {
 		];
 
 		foreach ( $logparams as $logKey ) {
+			if (
+				$logKey === 'log_user_text' &&
+				!LogEventsList::userCan(
+					$rowMain, LogPage::DELETED_USER, $user
+				)
+			) {
+				continue;
+			}
 			$query[$keys[$logKey]] = $matchedRows[0]->$logKey;
 		}
 		$query['offset'] = $query['offset'] + 1;
-- 
2.8.1