Loading data for an IP that has no edits (e.g. here) results in 26 seconds of MySQL query time. I think that is mitigated due to the access controls on that endpoint, but we should still investigate and fix it as soon as possible.
Description
Description
Details
Details
- Risk Rating
- Medium
- Author Affiliation
- WMF Product
Related Changes in Gerrit:
Customize query in gerrit
Related Objects
Related Objects
- Mentioned In
- T380221: Allow authorised users to see IP Info for actors if the IP exists in the CheckUser or AbuseFilter tables, or has contributions
T389312: Write and send supplementary release announcement for extensions and skins with security patches (1.39.13/1.42.7/1.43.2)
T392966: IP Information tool no longer showing "IP information for this address cannot be retrieved since no edits have been made from it." but instead shows information - Mentioned Here
- T392172: 1.45.0-wmf.2 deployment blockers
Event Timeline
Comment Actions
The queries involved in checkIPisKnown are:
SELECT 1 FROM `cu_changes` FORCE INDEX (cuc_actor_ip_time) JOIN `actor` ON ((cuc_actor=actor_id)) WHERE actor_name = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7' LIMIT 1 SELECT 1 FROM `cu_log_event` FORCE INDEX (cule_actor_ip_time) JOIN `actor` ON ((cule_actor=actor_id)) WHERE actor_name = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7' LIMIT 1 SELECT 1 FROM `cu_private_event` FORCE INDEX (cupe_actor_ip_time) LEFT JOIN `actor` ON ((cupe_actor=actor_id)) WHERE ((actor_name = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7' OR (cupe_actor IS NULL AND cupe_ip = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7'))) LIMIT 1 SELECT 1 FROM `abuse_filter_log` FORCE INDEX (afl_ip_timestamp) WHERE afl_user_text = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7' LIMIT 1
These are the problem queries:
mysql:research@dbstore1008.eqiad.wmnet [enwiki]> SELECT 1 FROM `cu_private_event` FORCE INDEX (cupe_actor_ip_time) LEFT JOIN `actor` ON ((cupe_actor=actor_id)) WHERE ((actor_name = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7' OR (cupe_actor IS NULL AND cupe_ip = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7'))) LIMIT 1; +---+ | 1 | +---+ | 1 | +---+ 1 row in set (58.324 sec) mysql:research@dbstore1008.eqiad.wmnet [enwiki]> EXPLAIN SELECT 1 FROM `cu_private_event` FORCE INDEX (cupe_actor_ip_time) LEFT JOIN `actor` ON ((cupe_actor=actor_id)) WHERE ((actor_name = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7' OR (cupe_actor IS NULL AND cupe_ip = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7'))) LIMIT 1; +------+-------------+------------------+--------+--------------------+--------------------+---------+------------------------------------+---------+-------------+ | id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra | +------+-------------+------------------+--------+--------------------+--------------------+---------+------------------------------------+---------+-------------+ | 1 | SIMPLE | cu_private_event | index | cupe_actor_ip_time | cupe_actor_ip_time | 281 | NULL | 5102153 | Using index | | 1 | SIMPLE | actor | eq_ref | PRIMARY | PRIMARY | 8 | enwiki.cu_private_event.cupe_actor | 1 | Using where | +------+-------------+------------------+--------+--------------------+--------------------+---------+------------------------------------+---------+-------------+ 2 rows in set (0.001 sec)
mysql:research@dbstore1008.eqiad.wmnet [enwiki]> SELECT 1 FROM `abuse_filter_log` FORCE INDEX (afl_ip_timestamp) WHERE afl_user_text = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7' LIMIT 1
-> ;
+---+
| 1 |
+---+
| 1 |
+---+
1 row in set (42.377 sec)
mysql:research@dbstore1008.eqiad.wmnet [enwiki]> EXPLAIN SELECT 1 FROM `abuse_filter_log` FORCE INDEX (afl_ip_timestamp) WHERE afl_user_text = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7' LIMIT 1;
+------+-------------+------------------+------+---------------+------+---------+------+----------+-------------+
| id | select_type | table | type | possible_keys | key | key_len | ref | rows | Extra |
+------+-------------+------------------+------+---------------+------+---------+------+----------+-------------+
| 1 | SIMPLE | abuse_filter_log | ALL | NULL | NULL | NULL | NULL | 39928601 | Using where |
+------+-------------+------------------+------+---------------+------+---------+------+----------+-------------+
1 row in set (0.001 sec)Comment Actions
When running the following on my local wiki, it says the possible_keys is afl_user_timestamp. It seems like the wrong index was used for that query.
EXPLAIN SELECT 1 FROM `abuse_filter_log` WHERE afl_user_text = '2600:8800:12C1:3300:BD2E:63A5:5135:17D7' LIMIT 1;
I'm not sure what is exactly broken in the cu_private_event query at this point.
Comment Actions
It seems that in this patch the IP is being used as the actor ID for the query to cu_private_event? Is this intentional?
Comment Actions
Now with commit message. Sorry about that; I think I read the wrong set of documentation.
Comment Actions
Follow-ups after testing on staging:
- actor id query was returning something unintended
- abuse filter query was still slow due to incorrect index
This needs to be deployed to .25, where the original patch was deployed to and tested against.
Comment Actions
Is this something the Security-Team should plan to discretely deploy this week? I guess we'd want to determine the sense of urgency here (e.g. how likely it is that an attacker could exploit this over the next few days).
If this works for 1.44.0-wmf.27 and beyond, and this isn't an urgent issue, I'd recommend we deploy this during next Monday's (May 5th) security deployment window.
Comment Actions
Sorry I forgot to follow up here. Both wmf.27 and wmf.25 are patched. The patches are in /srv/patches. So I think the next step here is to update master in IPInfo via a gerrit patch, when you are ready to do that.
Comment Actions
Change #1143141 had a related patch set uploaded (by Kosta Harlan; author: STran):
[mediawiki/extensions/IPInfo@master] SECURITY: Improve slow queries in AnonymousUserIPLookup->checkIPIsKnown
Comment Actions
Change #1143141 merged by jenkins-bot:
[mediawiki/extensions/IPInfo@master] SECURITY: Improve slow queries in AnonymousUserIPLookup->checkIPIsKnown
Comment Actions
Change #1143191 had a related patch set uploaded (by Dreamy Jazz; author: STran):
[mediawiki/extensions/IPInfo@REL1_44] SECURITY: Improve slow queries in AnonymousUserIPLookup->checkIPIsKnown
Comment Actions
Change #1143191 merged by jenkins-bot:
[mediawiki/extensions/IPInfo@REL1_44] SECURITY: Improve slow queries in AnonymousUserIPLookup->checkIPIsKnown
Comment Actions
@STran In the case that the IP exists in the actor table, should the query also include a condition for when cupe_actor is null and the cupe_ip matches? Otherwise we miss cases such as when an IP makes a failed login attempt.
Comment Actions
Change #1146664 had a related patch set uploaded (by STran; author: STran):
[mediawiki/extensions/IPInfo@master] Always check cu_private_event in AnonymousUserIPLookup->checkIPIsKnown
Comment Actions
Change #1146664 merged by jenkins-bot:
[mediawiki/extensions/IPInfo@master] Always check cu_private_event in AnonymousUserIPLookup->checkIPIsKnown
Comment Actions
I have compared the output of ipinfo/v0/norevision for lots of IPs on my local docker wiki before and after the changes in this task. Since T392976#10827501, I have detected no differences.
I have not seen a query called from checkIPIsKnown which took more than 10ms.
Comment Actions
Patch 01-T392976.patch is currently failing to apply for the most recent code in the mainline branch of extensions/IPInfo. This is blocking MediaWiki release 1.45.0-wmf.2(T392172)
If the patch needs to be rebased
To unblock the release, a new version of the patch can be placed at the right location in the deployment server with the following Scap command:
REVISED_PATCH=<path_to_revised_patch> scap update-patch --message-body 'Rebase to solve merge conflicts with mainline code' /srv/patches/1.45.0-wmf.2/extensions/IPInfo/01-T392976.patch "$REVISED_PATCH"
If the patch has been made public
To unblock the release, the patch can be removed for the right version from the deployment server with the following Scap command:
scap remove-patch --message-body 'Remove patch already made public' /srv/patches/1.45.0-wmf.2/extensions/IPInfo/01-T392976.patch
(Note that if patches for the version don't exist yet, they will be created and the patch you specified removed)
Comment Actions
Mentioned in SAL (#wikimedia-operations) [2025-05-18T12:50:21Z] <sbassett> Ran scap remove-patch for T392976
Comment Actions
Change #1165043 had a related patch set uploaded (by Mmartorana; author: STran):
[mediawiki/extensions/IPInfo@REL1_43] SECURITY: Improve slow queries in AnonymousUserIPLookup->checkIPIsKnown
Comment Actions
Change #1165045 had a related patch set uploaded (by Mmartorana; author: STran):
[mediawiki/extensions/IPInfo@REL1_44] Always check cu_private_event in AnonymousUserIPLookup->checkIPIsKnown
Comment Actions
Change #1165045 merged by jenkins-bot:
[mediawiki/extensions/IPInfo@REL1_44] Always check cu_private_event in AnonymousUserIPLookup->checkIPIsKnown
Comment Actions
Change #1165043 abandoned by SBassett:
[mediawiki/extensions/IPInfo@REL1_43] SECURITY: Improve slow queries in AnonymousUserIPLookup->checkIPIsKnown
Reason:
The affected code seems to only be from REL1_44 forward (see also: https://github.com/wikimedia/mediawiki-extensions-IPInfo/tree/REL1_43)