From c8f4978c507c921a597b972872da54beb18c4d68 Mon Sep 17 00:00:00 2001
From: Daimona Eaytoy <daimona.wiki@gmail.com>
Date: Wed, 18 Sep 2019 15:25:31 +0200
Subject: [PATCH] SECURITY: Avoid info leaks in ApiAbuseFilterCheckMatch

There are various info leaks for both deleted rc rows, and suppressed
AbuseLog entries.

Bug: T223654
Change-Id: I86ee3c13cc93097ceec38b9adf0c8acf501ddddc
---
 i18n/api/en.json                          |  4 +++-
 i18n/api/qqq.json                         |  4 +++-
 includes/AbuseFilterChangesList.php       |  1 +
 includes/api/ApiAbuseFilterCheckMatch.php | 27 +++++++++++++++++++++--
 4 files changed, 32 insertions(+), 4 deletions(-)

diff --git a/i18n/api/en.json b/i18n/api/en.json
index 609facb3..0638720d 100644
--- a/i18n/api/en.json
+++ b/i18n/api/en.json
@@ -59,5 +59,7 @@
 	"apierror-abusefilter-cantcheck": "You don't have permission to check syntax of abuse filters.",
 	"apierror-abusefilter-canteval": "You don't have permission to evaluate AbuseFilter expressions.",
 	"apierror-abusefilter-nosuchlogid": "There is no abuselog entry with the id $1.",
-	"apierror-abusefilter-badsyntax": "The filter has invalid syntax."
+	"apierror-abusefilter-badsyntax": "The filter has invalid syntax.",
+	"apierror-cannottest-deleted-rc": "You don't have permission to test abuse filters against the provided recent change because it's hidden from public view.",
+	"apierror-cannottest-deleted-abuselog": "You don't have permission to test abuse filters against the provided AbuseLog entry because it's hidden from public view."
 }
diff --git a/i18n/api/qqq.json b/i18n/api/qqq.json
index c8cefc26..e38e10a6 100644
--- a/i18n/api/qqq.json
+++ b/i18n/api/qqq.json
@@ -91,5 +91,7 @@
 	"apierror-abusefilter-cantcheck": "{{doc-apierror}}",
 	"apierror-abusefilter-canteval": "{{doc-apierror}}",
 	"apierror-abusefilter-nosuchlogid": "{{doc-apierror}}\n\nParameters:\n* $1 - AbuseFilter log ID number.",
-	"apierror-abusefilter-badsyntax": "{{doc-apierror}}"
+	"apierror-abusefilter-badsyntax": "{{doc-apierror}}",
+	"apierror-cannottest-deleted-rc": "{{doc-apierror}}",
+	"apierror-cannottest-deleted-abuselog": "{{doc-apierror}}"
 }
diff --git a/includes/AbuseFilterChangesList.php b/includes/AbuseFilterChangesList.php
index 86638aaa..9ba7c3b3 100644
--- a/includes/AbuseFilterChangesList.php
+++ b/includes/AbuseFilterChangesList.php
@@ -26,6 +26,7 @@ class AbuseFilterChangesList extends OldChangesList {
 		if ( (int)$rc->getAttribute( 'rc_deleted' ) !== 0 ) {
 			$s .= ' ' . $this->msg( 'abusefilter-log-hidden-implicit' )->parse();
 			if ( !$this->userCan( $rc, Revision::SUPPRESSED_ALL ) ) {
+				// Remember to keep this in sync with ApiAbuseFilterCheckMatch
 				return;
 			}
 		}
diff --git a/includes/api/ApiAbuseFilterCheckMatch.php b/includes/api/ApiAbuseFilterCheckMatch.php
index 8f0251f5..3c4f61ee 100644
--- a/includes/api/ApiAbuseFilterCheckMatch.php
+++ b/includes/api/ApiAbuseFilterCheckMatch.php
@@ -1,15 +1,18 @@
 <?php
 
+use MediaWiki\Revision\RevisionRecord;
+
 class ApiAbuseFilterCheckMatch extends ApiBase {
 	/**
 	 * @see ApiBase::execute
 	 */
 	public function execute() {
+		$user = $this->getUser();
 		$params = $this->extractRequestParams();
 		$this->requireOnlyOneParameter( $params, 'vars', 'rcid', 'logid' );
 
 		// "Anti-DoS"
-		if ( !AbuseFilter::canViewPrivate( $this->getUser() ) ) {
+		if ( !AbuseFilter::canViewPrivate( $user ) ) {
 			$this->dieWithError( 'apierror-abusefilter-canttest', 'permissiondenied' );
 		}
 
@@ -33,12 +36,26 @@ class ApiAbuseFilterCheckMatch extends ApiBase {
 				$this->dieWithError( [ 'apierror-nosuchrcid', $params['rcid'] ] );
 			}
 
+			$type = (int)$row->rc_type;
+			if (
+				(
+					$type === RC_LOG &&
+					!LogEventsList::userCanBitfield( $row->rc_deleted, Revision::SUPPRESSED_ALL, $user )
+				) || (
+					$row->rc_type !== RC_LOG &&
+					!RevisionRecord::userCanBitfield( $row->rc_deleted, Revision::SUPPRESSED_ALL, $user )
+				)
+			) {
+				// T223654 - Same check as in AbuseFilterChangesList
+				$this->dieWithError( 'apierror-cannottest-deleted-rc', 'deletedrc' );
+			}
+
 			$vars = AbuseFilter::getVarsFromRCRow( $row );
 		} elseif ( $params['logid'] ) {
 			$dbr = wfGetDB( DB_REPLICA );
 			$row = $dbr->selectRow(
 				'abuse_filter_log',
-				'afl_var_dump',
+				[ 'afl_var_dump', 'afl_deleted', 'afl_rev_id' ],
 				[ 'afl_id' => $params['logid'] ],
 				__METHOD__
 			);
@@ -47,6 +64,12 @@ class ApiAbuseFilterCheckMatch extends ApiBase {
 				$this->dieWithError( [ 'apierror-abusefilter-nosuchlogid', $params['logid'] ], 'nosuchlogid' );
 			}
 
+			if ( !SpecialAbuseLog::canSeeHidden( $user ) && SpecialAbuseLog::isHidden( $row ) ) {
+				// T223654 - Same check as in SpecialAbuseLog. Both the visibility of the AbuseLog entry
+				// and the corresponding revision are checked.
+				$this->dieWithError( 'apierror-cannottest-deleted-abuselog', 'deletedabuselog' );
+			}
+
 			$vars = AbuseFilter::loadVarDump( $row->afl_var_dump );
 		}