Page MenuHomePhabricator

Private filters should not be visible in recent changes
Open, LowPublic

Description

When a vandal is active, and someone creates/changes a private abusefilter to stop him, there will be a log event in recent changes, available for the vandal to see. This can draw unwanted attention to the feature. Such log events should be hidden from users without the abusefilter-viewprivate right.

Details

Reference
bz32959
Related Gerrit Patches:

Event Timeline

bzimport raised the priority of this task from to Medium.Nov 22 2014, 12:00 AM
bzimport added a project: AbuseFilter.
bzimport set Reference to bz32959.
bzimport added a subscriber: Unknown Object (MLST).
Tgr created this task.Dec 11 2011, 8:39 AM
Pine added a comment.Jun 5 2012, 4:31 AM

How would we prevent the vandal from seeing logs that are set to public? This is the nature of public abusefilters.

I'm not sure this is a bug. If you look at [[Special:Log/abusefilter]], you can't see any information besides timestamp, user, and filter number for a private filter. None of that information should be private really.

(In reply to comment #2)

How would we prevent the vandal from seeing logs that are set to public? This
is the nature of public abusefilters.

I think the bug only covers private filters.

Tgr added a comment.Dec 14 2012, 7:48 PM

The point is that a vandal who, until then, maybe did not even know that there are such things as edit filters, will easily realize that he is being tracked by such a filter by looking at the Recent Changes page. Vandals usually do not look at system logs, but they do look at RC.

(In reply to comment #4)

The point is that a vandal who, until then, maybe did not even know that
there
are such things as edit filters, will easily realize that he is being
tracked
by such a filter by looking at the Recent Changes page.

How would the vandal know that that specific filter was for xyr if they can only see the filter #? And wouldn't a vandal know they are being tracked as soon as they see that their edits are being blocked by the filter?

Vandals usually do
not
look at system logs, but they do look at RC.

I'm not seeing how hiding this information in once place but not in another is advantageous, and will prevent exploitation of the filters.

Tgr added a comment.Dec 14 2012, 9:23 PM

(In reply to comment #5)

How would the vandal know that that specific filter was for xyr if they can
only see the filter #?

From the timing, especially on smaller wikis. If you are vandalizing, your edits are refused, you change your pattern, you get through, but after a few edits you are refused again, and the same time an RC entry appears about abuse filter changes, that is kind of obvious.

And wouldn't a vandal know they are being tracked as
soon as they see that their edits are being blocked by the filter?

Sure, but the filter messages in themselves do not give you much information about what is going on (not enough to abuse it, anyway). Pointing them to the AbuseFilter page, which further points them to the documentation, with details of how exactly the tool works, is unhelpful.

I'm not seeing how hiding this information in once place but not in another
is advantageous, and will prevent exploitation of the filters.

Ideally it should be hidden everywhere (given that the log does not say anything useful, and all the links do not work if you have no privileges, there is no point to showing them anyway - it is bad UX design to put links before the user and only notify him after he clicks that he is not allowed to use them), but hiding it on the page which abusers are reading is more important than hiding it on pages they are not reading.

Ok, I think it makes enough sense to work on this.

I looked into it, and MediaWiki itself doesn't support hiding log entries to a certain usergroup, so a patch to MediaWiki core would needed to support this.

Luis150902 moved this task from Backlog to Filter privacy on the AbuseFilter board.Apr 2 2018, 5:10 PM
Daimona updated the task description. (Show Details)May 5 2018, 5:53 PM
Daimona added a subscriber: Daimona.May 5 2018, 5:56 PM

This is actually doable, we only need to set a LogRestriction on abusefilter log (the one in Special:Log for creations/edits). The question is: who should be able to see it? To me abusefilter-view-private sounds like an exaggeration, while abusefilter-view may be reasonable.

Vedmaka added a subscriber: Vedmaka.Sep 9 2018, 6:21 PM
DannyS712 added a subscriber: DannyS712.

This is actually doable, we only need to set a LogRestriction on abusefilter log (the one in Special:Log for creations/edits). The question is: who should be able to see it? To me abusefilter-view-private sounds like an exaggeration, while abusefilter-view may be reasonable.

Claiming to add the requirement that users have abusefilter-view in order to be able to view abusefilter log entries (log entries in Special:Log, not in Special:AbuseLog)

Restricted Application added a project: User-DannyS712. · View Herald TranscriptSep 15 2019, 10:35 PM

This is actually doable, we only need to set a LogRestriction on abusefilter log (the one in Special:Log for creations/edits). The question is: who should be able to see it? To me abusefilter-view-private sounds like an exaggeration, while abusefilter-view may be reasonable.

Claiming to add the requirement that users have abusefilter-view in order to be able to view abusefilter log entries (log entries in Special:Log, not in Special:AbuseLog)

Yes, probably. Sounds like the best choice. However, with regard to my first comment, I should note that it won't really solve the problem described in this task. Restricting Special:Log/abusefilter to abusefilter-view is IMHO necessary, and will help mitigate the issue (especially for anons). But there's no way (AFAIK) to specify different visibility levels for different log entries.

Change 550010 had a related patch set uploaded (by DannyS712; owner: DannyS712):
[mediawiki/extensions/AbuseFilter@master] Restrict viewing Special:Log/AbuseFilter, and remove from recent changes

https://gerrit.wikimedia.org/r/550010

Leaving open in case someone is interested in exploring the possibility to further restrict log entries referring to private filters to users with the view-private right. That doesn't seem feasible ATM, though.

Change 550010 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] Restrict viewing Special:Log/AbuseFilter, and remove from recent changes

https://gerrit.wikimedia.org/r/550010

Change 552650 had a related patch set uploaded (by Daimona Eaytoy; owner: DannyS712):
[mediawiki/extensions/AbuseFilter@wmf/1.35.0-wmf.5] Restrict viewing Special:Log/AbuseFilter, and remove from recent changes

https://gerrit.wikimedia.org/r/552650

Change 552650 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@wmf/1.35.0-wmf.5] Restrict viewing Special:Log/AbuseFilter, and remove from recent changes

https://gerrit.wikimedia.org/r/552650

Mentioned in SAL (#wikimedia-operations) [2019-11-25T11:16:24Z] <urbanecm@deploy1001> Synchronized php-1.35.0-wmf.5/extensions/AbuseFilter/extension.json: SWAT: 29a16bd: Restrict viewing Special:Log/AbuseFilter, and remove from recent changes (T34959) (duration: 01m 04s)

DannyS712 closed this task as Resolved.Mon, Nov 25, 11:25 AM
DannyS712 removed a project: Patch-For-Review.

I'd rather leave this open per T34959#5650620. We could also create another task, but I don't think it'd be better, given all of the things that were discussed here, and the title itself of this task.

Daimona reopened this task as Open.Mon, Nov 25, 12:01 PM

So, another partial solution is to hide the ID if the filter is private. We'd need another (generic) message, and we would decide which one to use in getMessageKey, by using $this->context->getUser() and filterHidden. However, we'd still:

  • show the log subtype
  • show something (and sometimes this would already be "exploitable")
  • not be able to judge whether the filter was hidden at the time it was triggered (i.e. it would be considered hidden iff it's currently hidden)

The best solution would involve changing LogPager in core. We cannot simply act at the LogFormatter level, because IndexPager::getBody() produces a line for each entry returned by the DB query (via LogPager::formatRow). So that's where we need to act. I see two possible ways forward:

  • Offer a way to alter the query params, e.g. a hook. This way extensions will be able to add some filtering, but only for things that can be achieved with pure SQL. It wouldn't suffice for this use case.
  • Offer a way to exclude a given row (this can also be implemented via hooks). This would suffice for most of the use cases, included ours. However, it has a significant downside: by the time we have a row available, the DB has already been queried, and we've already calculated the limits for paging. If one or more rows are excluded, then we'd show fewer results than we were asked for. In theory, it's possible to top up the result set with extra rows, but that could behave very badly, especially for large tables. Something similar was discussed (and later implemented) for Special:AbuseFilter at https://gerrit.wikimedia.org/r/#/c/mediawiki/extensions/AbuseFilter/+/538026/. However, while the abuse_filter table has roughly 1000 rows on enwiki, the logging table has 86 millions. We could end up with full table scans even with batching, and that could lead to any sort of problem.
Tgr added a comment.Mon, Nov 25, 8:02 PM

The only sane solution IMO is to push the required data into SQL (a private flag in log_search + a maintenance script to backfill) and allow some hook to filter based on that. Not sure if it's worth the effort.

Daimona lowered the priority of this task from Medium to Low.Tue, Nov 26, 1:01 PM
In T34959#5691124, @Tgr wrote:

The only sane solution IMO is to push the required data into SQL (a private flag in log_search + a maintenance script to backfill) and allow some hook to filter based on that.

Right. I think this would be perfect for our use case, but:

Not sure if it's worth the effort.

I don't think it is, hence lowering priority. The partial solution at T34959#5689966 would be OK as a stopgap, but aside from that, I think we shouldn't worry about this.

DannyS712 removed DannyS712 as the assignee of this task.Tue, Nov 26, 9:07 PM