Page MenuHomePhabricator
Files F26623496

T207085.patch
Daimona
Actions

Authored By
Daimona
Oct 18 2018, 5:03 PM
Size
8 KB
Referenced Files
None
Subscribers
None

T207085.patch
View Options

From 523f2d25c9f9636a4f6fb180544378e3f83e0164 Mon Sep 17 00:00:00 2001
From: Daimona Eaytoy <daimona.wiki@gmail.com>
Date: Tue, 16 Oct 2018 23:04:20 +0200
Subject: [PATCH] Remove info leak
Oversighted/deleted edits and log actions were entirely accessible to
non-oversighters via AbuseFilter/examine for RC, and via AbuseFilter/test.
Now, we take into account the revision/log visibility and user permissions to
determine what to show.
Other changes in this patch:
*Show the examine link if and only if the user can examine the given row
*If a revision is hidden but the user can see it, don't hide its elements in
ChangesList (only leave them striked/greyed)
*Make APIs better understand revision visibility.
*Make a clear distinction between deleted and suppressed edits/log
entries.
Co-authored with rxy <git@rxy.jp>
Bug: T207085
Change-Id: Icfa48e366a7e5e3abd5d2155ecfddfc09b378088
---
includes/AbuseFilterChangesList.php | 65 +++++++++++++++++++++
includes/Views/AbuseFilterViewExamine.php | 15 +++--
includes/api/ApiQueryAbuseLog.php | 17 +++---
includes/pagers/AbuseFilterExaminePager.php | 1 -
includes/special/SpecialAbuseLog.php | 12 ++--
5 files changed, 92 insertions(+), 18 deletions(-)
diff --git a/includes/AbuseFilterChangesList.php b/includes/AbuseFilterChangesList.php
index 139e01ba..94855f34 100644
--- a/includes/AbuseFilterChangesList.php
+++ b/includes/AbuseFilterChangesList.php
@@ -23,6 +23,13 @@ class AbuseFilterChangesList extends OldChangesList {
* @suppress PhanUndeclaredProperty for $rc->filterResult, which isn't a big deal
*/
public function insertExtra( &$s, &$rc, &$classes ) {
+ if ( (int)$rc->getAttribute( 'rc_deleted' ) !== 0 ) {
+ $s .= ' ' . $this->msg( 'abusefilter-log-hidden-implicit' )->parse();
+ if ( !$this->userCan( $rc, Revision::SUPPRESSED_ALL ) ) {
+ return;
+ }
+ }
+
$examineParams = [];
if ( $this->testFilter ) {
$examineParams['testfilter'] = $this->testFilter;
@@ -48,6 +55,64 @@ class AbuseFilterChangesList extends OldChangesList {
}
}
+ /**
+ * Insert links to user page, user talk page and eventually a blocking link.
+ * Like the parent, but don't hide details if user can see them.
+ *
+ * @param string &$s HTML to update
+ * @param RecentChange &$rc
+ */
+ public function insertUserRelatedLinks( &$s, &$rc ) {
+ $links = $this->getLanguage()->getDirMark() . Linker::userLink( $rc->mAttribs['rc_user'],
+ $rc->mAttribs['rc_user_text'] ) .
+ Linker::userToolLinks( $rc->mAttribs['rc_user'], $rc->mAttribs['rc_user_text'] );
+
+ if ( $this->isDeleted( $rc, Revision::DELETED_USER ) ) {
+ if ( $this->userCan( $rc, Revision::DELETED_USER ) ) {
+ $s .= ' <span class="history-deleted">' . $links . '</span>';
+ } else {
+ $s .= ' <span class="history-deleted">' .
+ $this->msg( 'rev-deleted-user' )->escaped() . '</span>';
+ }
+ } else {
+ $s .= $links;
+ }
+ }
+
+ /**
+ * Insert a formatted comment. Like the parent, but don't hide details if user can see them.
+ * @param RecentChange $rc
+ * @return string
+ */
+ public function insertComment( $rc ) {
+ if ( $this->isDeleted( $rc, Revision::DELETED_COMMENT ) ) {
+ if ( $this->userCan( $rc, Revision::DELETED_COMMENT ) ) {
+ return ' <span class="history-deleted">' .
+ Linker::commentBlock( $rc->mAttribs['rc_comment'], $rc->getTitle() ) . '</span>';
+ } else {
+ return ' <span class="history-deleted">' .
+ $this->msg( 'rev-deleted-comment' )->escaped() . '</span>';
+ }
+ } else {
+ return Linker::commentBlock( $rc->mAttribs['rc_comment'], $rc->getTitle() );
+ }
+ }
+
+ /**
+ * Insert a formatted action. The same as parent, but with a different audience in LogFormatter
+ *
+ * @param RecentChange $rc
+ * @return string
+ */
+ public function insertLogEntry( $rc ) {
+ $formatter = LogFormatter::newFromRow( $rc->mAttribs );
+ $formatter->setContext( $this->getContext() );
+ $formatter->setAudience( LogFormatter::FOR_THIS_USER );
+ $formatter->setShowUserToolLinks( true );
+ $mark = $this->getLanguage()->getDirMark();
+ return $formatter->getActionText() . " $mark" . $formatter->getComment();
+ }
+
/**
* @param string &$s
* @param RecentChange &$rc
diff --git a/includes/Views/AbuseFilterViewExamine.php b/includes/Views/AbuseFilterViewExamine.php
index aec63b2c..6c3d065c 100644
--- a/includes/Views/AbuseFilterViewExamine.php
+++ b/includes/Views/AbuseFilterViewExamine.php
@@ -112,6 +112,11 @@ class AbuseFilterViewExamine extends AbuseFilterView {
return;
}
+ if ( !ChangesList::userCan( RecentChange::newFromRow( $row ), Revision::SUPPRESSED_ALL ) ) {
+ $out->addWikiMsg( 'abusefilter-log-details-hidden-implicit' );
+ return;
+ }
+
self::$examineType = 'rc';
self::$examineId = $rcid;
@@ -157,10 +162,12 @@ class AbuseFilterViewExamine extends AbuseFilterView {
return;
}
- if ( SpecialAbuseLog::isHidden( $row ) === 'implicit' &&
- !$this->getUser()->isAllowed( 'deletedtext' ) ) {
- $out->addWikiMsg( 'abusefilter-log-details-hidden-implicit' );
- return;
+ if ( SpecialAbuseLog::isHidden( $row ) === 'implicit' ) {
+ $rev = Revision::newFromId( $row->afl_rev_id );
+ if ( !$rev->userCan( Revision::SUPPRESSED_ALL, $this->getUser() ) ) {
+ $out->addWikiMsg( 'abusefilter-log-details-hidden-implicit' );
+ return;
+ }
}
$vars = AbuseFilter::loadVarDump( $row->afl_var_dump );
$out->addJsConfigVars( 'wgAbuseFilterVariables', $vars->dumpAllVars( true ) );
diff --git a/includes/api/ApiQueryAbuseLog.php b/includes/api/ApiQueryAbuseLog.php
index 31e06a25..5ddc8ccb 100644
--- a/includes/api/ApiQueryAbuseLog.php
+++ b/includes/api/ApiQueryAbuseLog.php
@@ -172,10 +172,14 @@ class ApiQueryAbuseLog extends ApiQueryBase {
$this->setContinueEnumParameter( 'start', $ts->getTimestamp( TS_ISO_8601 ) );
break;
}
- if ( SpecialAbuseLog::isHidden( $row ) &&
- !SpecialAbuseLog::canSeeHidden()
- ) {
+ $hidden = SpecialAbuseLog::isHidden( $row );
+ if ( $hidden === true && !SpecialAbuseLog::canSeeHidden() ) {
continue;
+ } elseif ( $hidden === 'implicit' ) {
+ $rev = Revision::newFromId( $row->afl_rev_id );
+ if ( !$rev->userCan( Revision::SUPPRESSED_ALL, $user ) ) {
+ continue;
+ }
}
$canSeeDetails = SpecialAbuseLog::canSeeDetails( $row->afl_filter );
@@ -230,11 +234,8 @@ class ApiQueryAbuseLog extends ApiQueryBase {
}
}
- if ( $fld_hidden ) {
- $val = SpecialAbuseLog::isHidden( $row );
- if ( $val ) {
- $entry['hidden'] = $val;
- }
+ if ( $fld_hidden && $hidden ) {
+ $entry['hidden'] = $hidden;
}
if ( $entry ) {
diff --git a/includes/pagers/AbuseFilterExaminePager.php b/includes/pagers/AbuseFilterExaminePager.php
index 767239b9..9a305a2a 100644
--- a/includes/pagers/AbuseFilterExaminePager.php
+++ b/includes/pagers/AbuseFilterExaminePager.php
@@ -54,7 +54,6 @@ class AbuseFilterExaminePager extends ReverseChronologicalPager {
* @return string
*/
public function formatRow( $row ) {
- // Incompatible stuff.
$rc = RecentChange::newFromRow( $row );
$rc->counter = $this->mPage->mCounter++;
return $this->mChangesList->recentChangesLine( $rc, false );
diff --git a/includes/special/SpecialAbuseLog.php b/includes/special/SpecialAbuseLog.php
index 4344d77d..f8d2d2d9 100644
--- a/includes/special/SpecialAbuseLog.php
+++ b/includes/special/SpecialAbuseLog.php
@@ -541,11 +541,13 @@ class SpecialAbuseLog extends SpecialPage {
$out->addWikiMsg( 'abusefilter-log-details-hidden' );
return;
- } elseif ( self::isHidden( $row ) === 'implicit' &&
- !$this->getUser()->isAllowed( 'deletedtext' ) ) {
+ } elseif ( self::isHidden( $row ) === 'implicit' ) {
+ $rev = Revision::newFromId( $row->afl_rev_id );
// The log is visible, but refers to a deleted revision
- $out->addWikiMsg( 'abusefilter-log-details-hidden-implicit' );
- return;
+ if ( !$rev->userCan( Revision::SUPPRESSED_ALL, $this->getUser() ) ) {
+ $out->addWikiMsg( 'abusefilter-log-details-hidden-implicit' );
+ return;
+ }
}
$output = Xml::element(
@@ -1100,7 +1102,7 @@ class SpecialAbuseLog extends SpecialPage {
*
* @param stdClass $row The abuse_filter_log row object.
*
- * @return Mixed true if the item is explicitly hidden, false if it is not.
+ * @return bool|string true if the item is explicitly hidden, false if it is not.
* The string 'implicit' if it is hidden because the corresponding revision is hidden.
*/
public static function isHidden( $row ) {
--
2.18.0.windows.1

File Metadata

Mime Type
text/x-diff
Storage Engine
blob
Storage Format
Raw Data
Storage Handle
6553667
Default Alt Text
T207085.patch (8 KB)

Event Timeline

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits