Page MenuHomePhabricator

AbuseFilter's Throttle consequence's user mode should treat separate IPs separately
Open, Needs TriagePublic

Description

Currently, AbuseFilter's Throttle consequence implements the user throttle mode by doing: $identifier = $user->getId();. This creates a shared throttle rule for all IP addresses, which doesn't look like a good thing to do.

I propose to use $identifier = $user->getName(); instead, which will not change anything for logged in users, but will treat each separate IP editor separately.

Other solution for that is to use user,ip throttling mode, which has also its own side effects (namely de-facto exempting users with highly dynamic ranges).

@Daimona would like to see this discussed in Phab first, which makes sense. So, what do you all think?

List of filters throttling only by user
wikifilter_id
acewiki1
acewiki2
acewiki3
arwiki96
arwiki131
arwiki175
azwiki26
azwikibooks1
betawikiversity6
bgwiki20
cawikinews15
commonswiki60
commonswiki110
commonswiki190
cswiki14
cswiki44
dewiki31
dewiki253
dewiki287
dewiki294
dewiki302
dewiki304
dewiki326
dewiki327
dewiktionary41
elwiki27
enwiki68
enwiki242
enwiki643
enwiki806
enwiki826
enwiki1135
enwiki1158
enwikibooks22
enwikibooks51
enwikiquote25
enwikiversity15
enwikivoyage18
enwiktionary44
enwiktionary83
enwiktionary133
eswiki121
eswikinews21
eswikivoyage24
fawiki17
fawiki43
fawiki106
fawiki167
fawiki169
fawiki185
fawiki186
fawiki187
fawiki192
fawiki205
fawikiquote31
fiwiki75
fiwiki163
frwiki214
frwiki283
frwiki326
frwiki332
frwikiquote2
frwikivoyage6
hewiki9
hewiki77
hewiki78
hewiki86
idwiki5
idwiki6
idwiki34
idwiki76
idwiki96
itwiki91
itwiki320
itwiki465
itwikivoyage14
jawiki163
mrwiki12
mrwiki51
mrwiki58
mrwiki144
plwiki49
plwikisource9
plwikisource10
ptwiki2
rowiki35
rowiki38
rowiki52
rowiki53
rowiki61
ruwiki16
ruwiki26
scowiki9
scowiki11
shwiki2
simplewiki49
simplewiki75
skwiki26
skwiki38
srwiki38
thwiki18
thwiki74
thwiki76
thwiki79
thwiki108
trwiki103
ukwiki29
ukwikivoyage29
viwiki4
viwiki31
viwiki86
viwiki102
viwiki104
wikidatawiki129
zh_yuewiki8
zh_yuewiki33
zhwiki8
zhwiki24
zhwiki43
zhwiki44
zhwiki91
zhwiki216
zhwiki317
zhwiki318
zhwiki326

(generated on 2021-08-30 by @Urbanecm via code from P17103)

Event Timeline

Change 715339 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/AbuseFilter@master] Throttle consequence: Use User::getName() rather than User::getId as an ID

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

There could be a filter which says "if a user adds this sensitive word more than twice in a minute, stop them" and the filter could be extended to anon-users. That filter would work now and prevent the anon user, even if the anon user tries IP hopping. But with this change, that filter will not work and (perhaps more importantly) there is no way to replicate that behavior after this change. This means a malicious user can evade rate-limited filters this way.

Before proceeding with this change, it would be worthwhile to run queries on WMF and get a list of all filters that have throttle by user (but not user,ip) enabled and have a member of Wikimedia-abusefilter-global-maintainers review them in this regard (I am happy to be the person who does it).

Thereafter, we can decide to proceed or not. If the decision is to proceed, including this on User-notice makes sense too.

In the meantime, we should also discuss whether we want to add back the option to throttle by all IPs treated as the same user. This doesn't have to be an either-or discussion.

There could be a filter which says "if a user adds this sensitive word more than twice in a minute, stop them" and the filter could be extended to anon-users. That filter would work now and prevent the anon user, even if the anon user tries IP hopping. But with this change, that filter will not work and (perhaps more importantly) there is no way to replicate that behavior after this change. This means a malicious user can evade rate-limited filters this way.

There sorta would be a way (the range mode). That only works on /24s, but...maybe that's sufficient in practice? You would also be able to create two filters, one for logged in users (throttled by user) and one for anonymous users (throttled by ip). Also not exact replacement, but might be close enough for most purposes.

Before proceeding with this change, it would be worthwhile to run queries on WMF and get a list of all filters that have throttle by user (but not user,ip) enabled and have a member of Wikimedia-abusefilter-global-maintainers review them in this regard (I am happy to be the person who does it).

Fortunately, that's not too hard to query. I ran P17104 on all active content wikis for that purpose, and P17102 is the result (pasted to description too). The script will not include filters which filter by user, and also something else (like page), because I didn't want to bother with those (it's bit harder to do in plain SQL).

Thereafter, we can decide to proceed or not. If the decision is to proceed, including this on User-notice makes sense too.

In the meantime, we should also discuss whether we want to add back the option to throttle by all IPs treated as the same user. This doesn't have to be an either-or discussion.

If we want to keep that behavior, I'd suggest to use some more obvious name (maybe userid?)

@Urbanecm great work so far! I will review these and get back to you here.

To the extent that I can understand these filters, most of them would not be negatively impacted (and many of them are seldom triggered anyway). Some even use other rules, such as action = move, that would preclude anonymous users.

I did not review all of them. But among the ones I did review, some that look like a potential concern include:

  • arwiki 175 (although I did not see any cross-ASN activity in its logs)
  • azwiki 26 (same as above)
  • dewiki 31: with this one, I actually did find evidence that the filter was able to block a user who was IP hopping (see logs and pay special attention to edits on Ritter Rost)
  • enwiki 242: (although I did not see any cross-ASN activity in its logs)

Given the above, I think we should start by a User-notice that proposes the idea and solicits feedback. If you know someone who is both active on dewiki as a sysop and on Phab, maybe we should solicit their feedback directly here too.