I hypothesize that this may be related to autoblock - I can explain further, but the information is protected by ANPDP, so I'll have to share it privately somehow. Didn't want to turn this into a security-restricted ticket and share it here right off the bat in case I'm wrong, since it really is just a guess.

I've created an NDA-protected paste with the explanation (h/t @AntiCompositeNumber for the suggestion) at P48918.

This is a serious DoS vector that affects blocking. Protecting.

First of all: I confirmed with @GeneralNotability over IRC that:

1. The issue was reproducible (several attempts to block the user were made)
2. The current solution (user blocked without autoblock enabled) is acceptable (this issue is not blocking an admin from blocking someone at this point).

According to Logstash, MediaWiki ran a SQL query, which failed to complete within a reasonable timeframe. A manual execution of the query took more than a minute (1 min 10 sec in particular). The query was:

SELECT  cuc_ip  FROM `cu_changes` JOIN `actor` ON ((actor_id=cuc_actor))   WHERE actor_user = 41830889  ORDER BY cuc_timestamp DESC LIMIT 1;

As @GeneralNotability said in P48918, this is happening because the user has a lot of edits, and scanning the cu_changes table. The problematic code is within CheckUser (in the onPerformRetroactiveAutoblock hook in particular; see trackback).

This query is significantly faster if the ordering is based on cuc_id (4 seconds) rather than cuc_timestamp (over a minute). Since cuc_id is consecutively assigned, it should roughly equal to the same thing, AFAICS.

I'm wondering whether this should be fixed by avoiding the ordering in the query (which is the slow part). Do we really need the last used IP? Maybe we really should just autoblock all the IPs?

FYI @Zabe @Dreamy_Jazz

I would not be surprised if the checkuser extension were sensitive to a similar DOS attack. If not in the PHP backend itself, then likely in the javascript which builds the summary table.

In T338276#8907804, @RoySmith wrote:

I would not be surprised if the checkuser extension were sensitive to a similar DOS attack. If not in the PHP backend itself, then likely in the javascript which builds the summary table.

Those results are limited to 5,000 IPs / UAs at most, but haven't tested how that would affect things.

In T338276#8907776, @Urbanecm wrote:

This query is significantly faster if the ordering is based on cuc_id (4 seconds) rather than cuc_timestamp (over a minute). Since cuc_id is consecutively assigned, it should roughly equal to the same thing, AFAICS.

I'm wondering whether this should be fixed by avoiding the ordering in the query (which is the slow part). Do we really need the last used IP? Maybe we really should just autoblock all the IPs?

FYI @Zabe @Dreamy_Jazz

I wonder if this query would run faster if it was forced to use the correct index? If I understand correctly, a similar SQL query with a force index statement is used by Special:CheckUser to generate the 'get edits' results. Has anyone tried running a CheckUser on the bot account and seeing if the query fails?

Using cuc_id as the ordering should be okay, but I'm unsure it would work well.

Autoblocking all IPs won't be a good idea. That could mean IPs used several months ago are autoblocked, which could have been reassigned. For highly dynamic IPs this could mean 50-100 IPs being autoblocked.

Okay. I've run Special:CheckUser on MalnadachBot:

• Get IPs:
  • Loads in a reasonable time.
• Get edits:
  • First page of results loads slowly, but does load without major issue (summary table lags the page a bit)
  • Asking for a second page by clicking "next 5,000" gives the following:

• Get users on a /64
  • First page loads very quickly.
  • Attempting to move to the second page by clicking "next 5,000" gives a DB error, but this DB error appears very quickly:

Trying with Special:Investigate the following occurs:

• Checking the user first loads the "IPs & User agents" tab which shows a banner indicating the results are limited, however, loads in a reasonable time

• Clicking "Timeline" gives the following error:

The explanation as to why this occurs is almost completely for the investigate and blocking issue because this bot account has made hundred of thousands of edits in the last 3 months.

In T338276#8908099, @Dreamy_Jazz wrote:

I wonder if this query would run faster if it was forced to use the correct index? If I understand correctly, a similar SQL query with a force index statement is used by Special:CheckUser to generate the 'get edits' results. Has anyone tried running a CheckUser on the bot account and seeing if the query fails?

Of course. For some reason I thought the query uses the correct index already. SELECT cuc_ip FROM cu_changes FORCE INDEX (cuc_actor_ip_time) JOIN actor ON ((actor_id=cuc_actor)) WHERE actor_user = 41830889 ORDER BY cuc_timestamp DESC LIMIT 1 works like a charm. Thanks!

Patch below:

Autoblocking all IPs won't be a good idea. That could mean IPs used several months ago are autoblocked, which could have been reassigned. For highly dynamic IPs this could mean 50-100 IPs being autoblocked.

Makes sense. A way to resolve this concern would be to use all IPs from the last day/week/similar, but given what you suggested above, I think that doesn't need to be changed here.

In T338276#8908143, @Urbanecm wrote:

Patch below:

T338276.patch878 BDownload

Testing this patch now.

In T338276#8908143, @Urbanecm wrote:

Patch below:

T338276.patch878 BDownload

+2

Adding Anti-Harassment as this affects Special:Investigate's Timeline mode.

In T338276#8908151, @Dreamy_Jazz wrote:

In T338276#8908143, @Urbanecm wrote:

Patch below:

T338276.patch878 BDownload

+2

Thanks. Deploying:

02:01 <urbanecm> !log Deploying security patch for T338276
02:01 <+stashbot> Logged the message at https://wikitech.wikimedia.org/wiki/Server_Admin_Log

Thanks all, for the quick response! Now tracking under T276237 and T333626.

In T338276#8908129, @Dreamy_Jazz wrote:

Okay. I've run Special:CheckUser on MalnadachBot:

• Get IPs:
  • Loads in a reasonable time.
• Get edits:
  • First page of results loads slowly, but does load without major issue (summary table lags the page a bit)
  • Asking for a second page by clicking "next 5,000" gives the following:

This has been fixed by T338287.

• Get users on a /64
  • First page loads very quickly.
  • Attempting to move to the second page by clicking "next 5,000" gives a DB error, but this DB error appears very quickly:

This has also been fixed by T338287.

Trying with Special:Investigate the following occurs:

• Checking the user first loads the "IPs & User agents" tab which shows a banner indicating the results are limited, however, loads in a reasonable time

• Clicking "Timeline" gives the following error:

The explanation as to why this occurs is almost completely for the investigate and blocking issue because this bot account has made hundred of thousands of edits in the last 3 months.

The Investigate issue has not been addressed, though a separate task for that issue would probably make sense. I will file another task for this.

All other issues raised when running checks have therefore been addressed. While the results do load slowly, they do not meet any limits. Addressing the speed of the checkuser helper table could probably be done by making the script be given the data pre-complied, so that it can be displayed instead of generated client side.

I've created T338419 for the remaining issue with Special:Investigate.

For this task, fixes should be backported for all currently supported release versions. Fixes for 1.35, 1.38 and 1.39 will need a modified patch as they did not have the SelectQueryBuilder to make queries and/or need to use a differently named index.

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

[mediawiki/extensions/CheckUser@master] SECURITY: Close a DoS vector by an index hint

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

Change 933629 had a related patch set uploaded (by Dreamy Jazz; author: Urbanecm):

[mediawiki/extensions/CheckUser@REL1_40] SECURITY: Close a DoS vector by an index hint

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

Change 933687 had a related patch set uploaded (by Dreamy Jazz; author: Urbanecm):

[mediawiki/extensions/CheckUser@REL1_39] SECURITY: Close a DoS vector by an index hint

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

Change 933688 had a related patch set uploaded (by Dreamy Jazz; author: Urbanecm):

[mediawiki/extensions/CheckUser@REL1_38] SECURITY: Close a DoS vector by an index hint

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

Change 933689 had a related patch set uploaded (by Dreamy Jazz; author: Urbanecm):

[mediawiki/extensions/CheckUser@REL1_35] SECURITY: Close a DoS vector by an index hint

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

Change 933629 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_40] SECURITY: Close a DoS vector by an index hint

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

Change 933688 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_38] SECURITY: Close a DoS vector by an index hint

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

Change 933687 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_39] SECURITY: Close a DoS vector by an index hint

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

Change 932823 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@master] SECURITY: Close a DoS vector by an index hint

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

Change 933689 merged by jenkins-bot:

[mediawiki/extensions/CheckUser@REL1_35] SECURITY: Close a DoS vector by an index hint

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

In T338276#8976902, @Dreamy_Jazz wrote:

Fix merged into the main branch and also backported to supported release versions. Therefore, this can be marked as resolved.

Any reason to keep this task private? I'm currently not seeing one.

In T338276#8977137, @sbassett wrote:

In T338276#8976902, @Dreamy_Jazz wrote:

Fix merged into the main branch and also backported to supported release versions. Therefore, this can be marked as resolved.

Any reason to keep this task private? I'm currently not seeing one.

I see no need to keep this private either.