From 6b720c114e8e0590ae68864b70cddd73b504411b Mon Sep 17 00:00:00 2001
From: Daimona Eaytoy <daimona.wiki@gmail.com>
Date: Sun, 3 Jan 2021 15:57:48 +0100
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: I1e8ff3968184763e661b03b989b5f94fc09d6698
---
 i18n/api/en.json                    |  4 +++-
 i18n/api/qqq.json                   |  4 +++-
 includes/AbuseFilterChangesList.php |  1 +
 includes/Api/CheckMatch.php         | 34 +++++++++++++++++++++++++++--
 4 files changed, 39 insertions(+), 4 deletions(-)

diff --git a/i18n/api/en.json b/i18n/api/en.json
index 8e46cd72..9b28af1d 100644
--- a/i18n/api/en.json
+++ b/i18n/api/en.json
@@ -62,5 +62,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-abusefilter-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-abusefilter-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 650dae6b..3cde2d48 100644
--- a/i18n/api/qqq.json
+++ b/i18n/api/qqq.json
@@ -94,5 +94,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-abusefilter-cannottest-deleted-rc": "{{doc-apierror}}",
+	"apierror-abusefilter-cannottest-deleted-abuselog": "{{doc-apierror}}"
 }
diff --git a/includes/AbuseFilterChangesList.php b/includes/AbuseFilterChangesList.php
index ba84d51b..dded8998 100644
--- a/includes/AbuseFilterChangesList.php
+++ b/includes/AbuseFilterChangesList.php
@@ -28,6 +28,7 @@ class AbuseFilterChangesList extends OldChangesList {
 		if ( (int)$rc->getAttribute( 'rc_deleted' ) !== 0 ) {
 			$s .= ' ' . $this->msg( 'abusefilter-log-hidden-implicit' )->parse();
 			if ( !$this->userCan( $rc, RevisionRecord::SUPPRESSED_ALL ) ) {
+				// Remember to keep this in sync with the CheckMatch API
 				return;
 			}
 		}
diff --git a/includes/Api/CheckMatch.php b/includes/Api/CheckMatch.php
index 4e426edf..8dc0b0d9 100644
--- a/includes/Api/CheckMatch.php
+++ b/includes/Api/CheckMatch.php
@@ -5,10 +5,14 @@ namespace MediaWiki\Extension\AbuseFilter\Api;
 use ApiBase;
 use ApiResult;
 use FormatJson;
+use LogEventsList;
 use LogicException;
+use LogPage;
 use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
+use MediaWiki\Extension\AbuseFilter\Special\SpecialAbuseLog;
 use MediaWiki\Extension\AbuseFilter\VariableGenerator\RCVariableGenerator;
 use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
+use MediaWiki\Revision\RevisionRecord;
 use RecentChange;
 
 class CheckMatch extends ApiBase {
@@ -17,11 +21,12 @@ class CheckMatch extends ApiBase {
 	 */
 	public function execute() {
 		$afPermManager = AbuseFilterServices::getPermissionManager();
+		$user = $this->getUser();
 		$params = $this->extractRequestParams();
 		$this->requireOnlyOneParameter( $params, 'vars', 'rcid', 'logid' );
 
 		// "Anti-DoS"
-		if ( !$afPermManager->canViewPrivateFilters( $this->getUser() ) ) {
+		if ( !$afPermManager->canViewPrivateFilters( $user ) ) {
 			$this->dieWithError( 'apierror-abusefilter-canttest', 'permissiondenied' );
 		}
 
@@ -36,9 +41,28 @@ class CheckMatch extends ApiBase {
 				$this->dieWithError( [ 'apierror-nosuchrcid', $params['rcid'] ] );
 			}
 
+			$type = (int)$rc->getAttribute( 'rc_type' );
+			$deletedValue = $rc->getAttribute( 'rc_deleted' );
+			if (
+				(
+					$type === RC_LOG &&
+					!LogEventsList::userCanBitfield(
+						$deletedValue,
+						LogPage::SUPPRESSED_ACTION | LogPage::SUPPRESSED_USER,
+						$user
+					)
+				) || (
+					$type !== RC_LOG &&
+					!RevisionRecord::userCanBitfield( $deletedValue, RevisionRecord::SUPPRESSED_ALL, $user )
+				)
+			) {
+				// T223654 - Same check as in AbuseFilterChangesList
+				$this->dieWithError( 'apierror-cannottest-deleted-rc', 'deletedrc' );
+			}
+
 			$vars = new VariableHolder();
 			// @phan-suppress-next-line PhanTypeMismatchArgumentNullable T240141
-			$varGenerator = new RCVariableGenerator( $vars, $rc, $this->getUser() );
+			$varGenerator = new RCVariableGenerator( $vars, $rc, $user );
 			$vars = $varGenerator->getVars();
 		} elseif ( $params['logid'] ) {
 			$dbr = wfGetDB( DB_REPLICA );
@@ -53,6 +77,12 @@ class CheckMatch extends ApiBase {
 				$this->dieWithError( [ 'apierror-abusefilter-nosuchlogid', $params['logid'] ], 'nosuchlogid' );
 			}
 
+			if ( !$afPermManager->canSeeHiddenLogEntries( $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 = AbuseFilterServices::getVariablesBlobStore()->loadVarDump( $row->afl_var_dump );
 		}
 		if ( $vars === null ) {