Last modified: 2012-01-21 02:56:06 UTC
If an action is caught by a private filter, it is entered into the log and by clicking the details link anyone can view the diff. If the filter's purpose is to prevent the posting of private information, this becomes a leak, e.g. if the filter matches a real name the diff will contain that real name. Therefore this information should either be private and only viewable by those who can view a private filter, or there should be an option when writing a filter to restrict the details view to those that can view the filter.
Marking "Highest" till we determine if this leakage is something we're really concerned with.
Created attachment 9768 [details] Add action hidedetails which automatically hides log entries This patch implements a restricted action called hidedetails that automatically hides the log entries created by the filter (i.e. sets afl_deleted to 1). Unfortunately this also creates an inconsistency in user rights - users with abusefilter-modify-restricted can create a filter that autohides its log entries, even though they might not have abusefilter-hide-log that is normally required to hide and unhide log entries. Please also see Bug 33390 to fully plug this leak.
r107454
I don't like this idea - being able to hide things from yourself is a rather large problem as far as being responsible for your own false positives goes. Reviewing false positives would require the right to see the logs, and (I believe) on some WMF wikis with abusefilter enabled there are no users with the right to see hidden abusefilter log entries.
Well, you need to have abusefilter-modify-restricted to be able to use this action in the first place, is that enabled on any WMF wiki?
Reverted in r107456 based on Prodego's comment. I'm reopening and really, really not touching this again for a bit since I don't know enough about AbuseFilter.
I didn't catch that it was added to the restricted list. There is already privilege inconsistency there with the degroup, block, and rangeblock abilities. So then this is a theoretical problem rather than a practical one, in a situation where lots of theoretical problems already exist. In that case, I was incorrect in saying this would be an immediate problem - it would just add to an existing mass of problems. I wouldn't mind including it then, though at some point, this whole thing should be reworked.
Created attachment 9771 [details] Fixed warning Previous patch caused a warning about an unrecognized action.
This is needed for private filters, there are some log entries o
which should not be visible for the public, especially as Nikola has stated. This is a practical problem unfortunately, not one in theory. A fast fix would be appreciated.
This cannot reasonably be enabled on WMF wikis for the reasons that Nikola and I both mentioned. A reworking of how the abusefilter handles restricted actions would be required before we can do that.
Ok, so on enwiki, oversighters can hide log entries, and sysops can view and edit private filters. So the solution seems to be that private filters should create log entries where the details and examine page are hidden from people who don't have abusefilter-view-private (the right required to view private filters, not to be confused with abusefilter-private, the right to view IP addresses in details, which is not enabled on enwiki). The oversighters can further hide those entries from sysops using the existing mechanism as well as hiding the entry completely from the abuse log.
Created attachment 9776 [details] Proposed patch 2 Ok, this patch makes log entries belonging to private/hidden filters behave as if the user did not have abusefilter-log-detail. I've added a parameter, $filter_id, to SpecialAbuseLog::canSeeDetails. If it's not null, the function checks if the filter is hidden (by calling AbuseFilter::filterHidden), and in that case returns true only if AbuseFilterView::canViewPrivate() (i.e. the user is allowed to view private filters) is true (in addition to requiring abusefilter-log-detail). I've made the methods canViewPrivate() and canEdit() of AbuseFilterView static, to avoid code duplication in the above. I hope that doesn't break anything, though it shouldn't since it already used a static variable. Since the abuse log may contain entries generated by global filters, I've modified AbuseFilter::filterHidden to handle global filters as well. I wasn't able to test this though. Whenever SpecialAbuseLog::canSeeDetails is called for a specific log entry, it is called with a filter id. However, it's called without a filter id for determining whether a user should be able to search for log entries belonging to a specific filter. This is allowed for public filters, but is disallowed for private ones. If the user tries to search for log entries belonging to a private filter, and canViewPrivate is false, the condition is not added to the query so it returns all log entries. In addition, I hid the hitcount from the filter list for private filters from users who cannot see details of the filter. The link would search for log entries created by the filter, but if the user can't view the details then that doesn't work (even before this patch, the links would be displayed but not work for users who didn't have abusefilter-log-detail). I don't know if the hitcount itself should be displayed without the link though, I opted not to.
Created attachment 9777 [details] Proposed patch 2 improved I got rid of some unnecessary db queries (when the function calling canSeeDetails already does a database query and has af_hidden, it passes it to canSeeDetails so it doesn't have to call filterHidden), and I changed the error message abusefilter-log-cannot-see-details, since it's shown when you try to view a single entry.
That's an excellent solution. I am trying to think if there would be a reason for the abuse log of private filters to be public, and I cannot immediately think of one.
Transparency. But another reason why details of private filters shouldn't be public is that they give insights into how the filter works. The point of having private filters is obfuscating the code, so that a spammer or whatever doesn't know what to avoid. Right now they have a convenient link to all the filter's previous hits, with a diff for each and every case where the filter prevented an edit, making it easier to figure out how to circumvent the filter.
Okay. If this patch works, how long will it take to implement/do we need to get some consensus somewhere? A private filter having a private log seems to be a no-brainer for me.
r107906
