wikimedia/mediawiki-extensions-AbuseFilter
1
0
Fork 0
mirror of https://gerrit.wikimedia.org/r/mediawiki/extensions/AbuseFilter.git synced 2024-11-24 06:03:49 +00:00
Code Issues Packages Projects Releases Wiki Activity

SECURITY: abusefiltercheckmatch: Check if user can see log details

Browse source 
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
This commit is contained in:
Dreamy Jazz 2024-09-11 18:20:49 +01:00 committed by Reedy
parent 990d5ff896
commit 233a4f1b31
2 changed files with 47 additions and 0 deletions
Show stats Download patch file Download diff file Expand all files Collapse all files

10
includes/Api/CheckMatch.php
View file

@ -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

37
tests/phpunit/integration/Api/CheckMatchTest.php
View file

@ -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' ] )
);
}
}