Page MenuHomePhabricator

T223654-api.patch

Authored By
Daimona
Jan 3 2021, 3:14 PM
Size
5 KB
Referenced Files
None
Subscribers
None

T223654-api.patch

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 ) {

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
8823919
Default Alt Text
T223654-api.patch (5 KB)

Event Timeline