From 233a4f1b31848baea222e62bb45cbbc65a146924 Mon Sep 17 00:00:00 2001
From: Dreamy Jazz <wpgbrown@wikimedia.org>
Date: Wed, 11 Sep 2024 18:20:49 +0100
Subject: [PATCH] SECURITY: abusefiltercheckmatch: Check if user can see log
 details

CVE-2024-PENDING

Why:
* The 'abusefiltercheckmatch' API allows callers to match
  arbitary filter conditions against existing AbuseFilter logs
* The API does not check if the performer has the ability to
  see the log details for the given filter, so can allow a user
  to bypass hidden and protected visibility settings.

What:
* Call AbuseFilterPermissionManager::canSeeLogDetailsForFilter
  before attempting to match a filter against a given AbuseFilter
  log.
* Add a test to verify that this security fix works.

Bug: T372998
Change-Id: I4a2467dc4e0d1f8401d5428a89c7f6d6ebcdfa70
---
 includes/Api/CheckMatch.php                   | 10 +++++
 .../integration/Api/CheckMatchTest.php        | 37 +++++++++++++++++++
 2 files changed, 47 insertions(+)

diff --git a/includes/Api/CheckMatch.php b/includes/Api/CheckMatch.php
index 7b6aa0ef6..f82c382f7 100644
--- a/includes/Api/CheckMatch.php
+++ b/includes/Api/CheckMatch.php
@@ -10,6 +10,7 @@ use LogEventsList;
 use LogicException;
 use LogPage;
 use MediaWiki\Extension\AbuseFilter\AbuseFilterPermissionManager;
+use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
 use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerFactory;
 use MediaWiki\Extension\AbuseFilter\Special\SpecialAbuseLog;
 use MediaWiki\Extension\AbuseFilter\VariableGenerator\VariableGeneratorFactory;
@@ -113,6 +114,15 @@ class CheckMatch extends ApiBase {
 				$this->dieWithError( [ 'apierror-abusefilter-nosuchlogid', $params['logid'] ], 'nosuchlogid' );
 			}
 
+			// TODO: Replace with dependency injection once security patch is uploaded publicly.
+			$afFilterLookup = AbuseFilterServices::getFilterLookup();
+			$privacyLevel = $afFilterLookup->getFilter( $row->afl_filter_id, $row->afl_global )
+				->getPrivacyLevel();
+			$canSeeDetails = $this->afPermManager->canSeeLogDetailsForFilter( $performer, $privacyLevel );
+			if ( !$canSeeDetails ) {
+				$this->dieWithError( 'apierror-permissiondenied-generic', 'cannotseedetails' );
+			}
+
 			$visibility = SpecialAbuseLog::getEntryVisibilityForUser( $row, $performer, $this->afPermManager );
 			if ( $visibility !== SpecialAbuseLog::VISIBILITY_VISIBLE ) {
 				// T223654 - Same check as in SpecialAbuseLog. Both the visibility of the AbuseLog entry
diff --git a/tests/phpunit/integration/Api/CheckMatchTest.php b/tests/phpunit/integration/Api/CheckMatchTest.php
index 20faccbea..9877708b9 100644
--- a/tests/phpunit/integration/Api/CheckMatchTest.php
+++ b/tests/phpunit/integration/Api/CheckMatchTest.php
@@ -4,16 +4,23 @@ namespace MediaWiki\Extension\AbuseFilter\Tests\Integration\Api;
 
 use ApiTestCase;
 use FormatJson;
+use MediaWiki\Extension\AbuseFilter\AbuseFilterServices;
+use MediaWiki\Extension\AbuseFilter\Filter\ExistingFilter;
+use MediaWiki\Extension\AbuseFilter\Filter\Flags;
+use MediaWiki\Extension\AbuseFilter\FilterLookup;
 use MediaWiki\Extension\AbuseFilter\Parser\Exception\InternalException;
 use MediaWiki\Extension\AbuseFilter\Parser\FilterEvaluator;
 use MediaWiki\Extension\AbuseFilter\Parser\ParserStatus;
 use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerFactory;
 use MediaWiki\Extension\AbuseFilter\Parser\RuleCheckerStatus;
+use MediaWiki\Extension\AbuseFilter\Variables\VariableHolder;
 use MediaWiki\Tests\Unit\Permissions\MockAuthorityTrait;
+use MediaWiki\Title\Title;
 
 /**
  * @coversDefaultClass \MediaWiki\Extension\AbuseFilter\Api\CheckMatch
  * @covers ::__construct
+ * @group Database
  * @group medium
  */
 class CheckMatchTest extends ApiTestCase {
@@ -97,4 +104,34 @@ class CheckMatchTest extends ApiTestCase {
 		] );
 	}
 
+	public function testExecuteWhenPerformerCannotSeeLogId() {
+		// Mock the FilterLookup service to return that the filter with the ID 1 is hidden.
+		$mockLookup = $this->createMock( FilterLookup::class );
+		$mockLookup->method( 'getFilter' )
+			->with( 1, false )
+			->willReturnCallback( function () {
+				$filterObj = $this->createMock( ExistingFilter::class );
+				$filterObj->method( 'getPrivacyLevel' )->willReturn( Flags::FILTER_HIDDEN );
+				return $filterObj;
+			} );
+		$this->setService( FilterLookup::SERVICE_NAME, $mockLookup );
+		// Create an AbuseFilter log entry for the hidden filter
+		AbuseFilterServices::getAbuseLoggerFactory()->newLogger(
+			Title::newFromText( 'Testing' ),
+			$this->getTestUser()->getUser(),
+			VariableHolder::newFromArray( [ 'action' => 'edit' ] )
+		)->addLogEntries( [ 1 => [ 'warn' ] ] );
+		// Execute the API using a user with the 'abusefilter-modify' right but without the
+		// 'abusefilter-log-detail' right, while specifying a filter abuse filter log ID of 1
+		$this->expectApiErrorCode( 'cannotseedetails' );
+		$this->doApiRequest(
+			[
+				'action' => 'abusefiltercheckmatch',
+				'logid' => 1,
+				'filter' => 'invalidfilter=======',
+			],
+			null, false, $this->mockRegisteredAuthorityWithPermissions( [ 'abusefilter-modify' ] )
+		);
+	}
+
 }