Page MenuHomePhabricator
Create Task
Maniphest T374686

GlobalBlockLookup generates invalid IP lookup conditions that select all rows when an IPv4 contains leading 0s
Closed, ResolvedPublic2 Estimated Story PointsPRODUCTION ERROR
Actions

Description

Error
normalized_message
Expectation (readQueryRows <= 10000) by MediaWiki\Actions\ActionEntryPoint::execute not met (actual: {actualSeconds}) in trx #{trxId}:
{query}
exception.trace
from /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/TransactionProfiler.php(534)
#0 /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/TransactionProfiler.php(327): Wikimedia\Rdbms\TransactionProfiler->reportExpectationViolated(string, Wikimedia\Rdbms\GeneralizedSql, int, string, string)
#1 /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/database/TransactionManager.php(593): Wikimedia\Rdbms\TransactionProfiler->recordQueryCompletion(Wikimedia\Rdbms\GeneralizedSql, float, bool, int, string, string)
#2 /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/database/Database.php(822): Wikimedia\Rdbms\TransactionManager->recordQueryCompletion(Wikimedia\Rdbms\GeneralizedSql, float, bool, int, string)
#3 /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/database/Database.php(707): Wikimedia\Rdbms\Database->attemptQuery(Wikimedia\Rdbms\Query, string, bool)
#4 /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/database/Database.php(634): Wikimedia\Rdbms\Database->executeQuery(Wikimedia\Rdbms\Query, string, int)
#5 /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/database/Database.php(1340): Wikimedia\Rdbms\Database->query(Wikimedia\Rdbms\Query, string)
#6 /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/database/DBConnRef.php(127): Wikimedia\Rdbms\Database->select(array, array, array, string, array, array)
#7 /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/database/DBConnRef.php(351): Wikimedia\Rdbms\DBConnRef->__call(string, array)
#8 /srv/mediawiki/php-1.43.0-wmf.22/includes/libs/rdbms/querybuilder/SelectQueryBuilder.php(746): Wikimedia\Rdbms\DBConnRef->select(array, array, array, string, array, array)
#9 /srv/mediawiki/php-1.43.0-wmf.22/extensions/GlobalBlocking/includes/Services/GlobalBlockLookup.php(297): Wikimedia\Rdbms\SelectQueryBuilder->fetchResultSet()
#10 /srv/mediawiki/php-1.43.0-wmf.22/extensions/GlobalBlocking/includes/Services/GlobalBlockLookup.php(171): MediaWiki\Extension\GlobalBlocking\Services\GlobalBlockLookup->getGlobalBlockingBlock(string, int, int)
#11 /srv/mediawiki/php-1.43.0-wmf.22/extensions/GlobalBlocking/includes/Services/GlobalBlockLookup.php(80): MediaWiki\Extension\GlobalBlocking\Services\GlobalBlockLookup->getUserBlockDetails(MediaWiki\User\User, string)
#12 /srv/mediawiki/php-1.43.0-wmf.22/extensions/GlobalBlocking/includes/GlobalBlockingHooks.php(109): MediaWiki\Extension\GlobalBlocking\Services\GlobalBlockLookup->getUserBlock(MediaWiki\User\User, null)
#13 /srv/mediawiki/php-1.43.0-wmf.22/includes/HookContainer/HookContainer.php(159): MediaWiki\Extension\GlobalBlocking\GlobalBlockingHooks->onGetUserBlock(MediaWiki\User\User, null, null)
#14 /srv/mediawiki/php-1.43.0-wmf.22/includes/HookContainer/HookRunner.php(2001): MediaWiki\HookContainer\HookContainer->run(string, array)
#15 /srv/mediawiki/php-1.43.0-wmf.22/includes/block/BlockManager.php(223): MediaWiki\HookContainer\HookRunner->onGetUserBlock(MediaWiki\User\User, null, null)
#16 /srv/mediawiki/php-1.43.0-wmf.22/includes/user/User.php(1450): MediaWiki\Block\BlockManager->getBlock(MediaWiki\User\User, null, bool)
#17 /srv/mediawiki/php-1.43.0-wmf.22/includes/user/User.php(1526): MediaWiki\User\User->getBlock()
#18 /srv/mediawiki/php-1.43.0-wmf.22/extensions/CheckUser/src/CheckUser/Pagers/CheckUserLogPager.php(174): MediaWiki\User\User->isHidden()
#19 /srv/mediawiki/php-1.43.0-wmf.22/includes/pager/ReverseChronologicalPager.php(134): MediaWiki\CheckUser\CheckUser\Pagers\CheckUserLogPager->formatRow(stdClass)
#20 /srv/mediawiki/php-1.43.0-wmf.22/includes/pager/IndexPager.php(595): MediaWiki\Pager\ReverseChronologicalPager->getRow(stdClass)
#21 /srv/mediawiki/php-1.43.0-wmf.22/extensions/CheckUser/src/CheckUser/SpecialCheckUserLog.php(158): MediaWiki\Pager\IndexPager->getBody()
#22 /srv/mediawiki/php-1.43.0-wmf.22/includes/specialpage/SpecialPage.php(719): MediaWiki\CheckUser\CheckUser\SpecialCheckUserLog->execute(null)
#23 /srv/mediawiki/php-1.43.0-wmf.22/includes/specialpage/SpecialPageFactory.php(1708): MediaWiki\SpecialPage\SpecialPage->run(null)
#24 /srv/mediawiki/php-1.43.0-wmf.22/includes/actions/ActionEntryPoint.php(502): MediaWiki\SpecialPage\SpecialPageFactory->executePath(string, MediaWiki\Context\RequestContext)
#25 /srv/mediawiki/php-1.43.0-wmf.22/includes/actions/ActionEntryPoint.php(145): MediaWiki\Actions\ActionEntryPoint->performRequest()
#26 /srv/mediawiki/php-1.43.0-wmf.22/includes/MediaWikiEntryPoint.php(200): MediaWiki\Actions\ActionEntryPoint->execute()
#27 /srv/mediawiki/php-1.43.0-wmf.22/index.php(58): MediaWiki\MediaWikiEntryPoint->run()
#28 /srv/mediawiki/w/index.php(3): require(string)
#29 {main}
Impact

Slow queries on Special:CheckUserLog for some old cu_log entries.

Notes

Appears to be caused by leading 0s in IP addresses used as the target for some old cu_log entries. Sanitising the IP address with IPUtils::sanitizeIP should fix the problem as it removes the leading 0s.

Details

Request URL
https://fr.wikipedia.org/w/index.php?cuInitiator=*&dir=*&offset=*&title=*
SubjectRepoBranchLines +/-
Sanitise IP in GlobalBlockLookup::getGlobalBlockLookupConditionsmediawiki/extensions/GlobalBlockingmaster+28 -8
Customize query in gerrit

Related Objects

Event Timeline

kostajh created this task.Sep 13 2024, 7:12 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptSep 13 2024, 7:12 AM
mszabo added a comment.Sep 13 2024, 9:11 AM

Both gb_range_start and gb_range_end are 0 in this query—likely a bug.

Dreamy_Jazz removed a project: CheckUser.Sep 13 2024, 9:55 AM
Dreamy_Jazz added a project: CheckUser.
Dreamy_Jazz added a comment.Sep 13 2024, 10:34 AM

The query, as noted by mszabo above, contains gb_range_start and gb_range_end as 0. Furthermore, the LIKE query is invalid (it selects all rows).

Looking further at the stack trace, we see that GlobalBlockLookup::getUserBlockDetails is passed a null for the $ip argument. Then ::getGlobalBlockingBlock is passed a string for the $ip argument. The only code that changes the $ip in ::getUserBlockDetails is https://gerrit.wikimedia.org/g/mediawiki/extensions/GlobalBlocking/+/803dac243b1fd4e2fabc13de5472e9167656d71c/includes/Services/GlobalBlockLookup.php#149. This suggests that the query is doing this invalid check for an IP address that is considered valid by IPUtils::isIPAddress but then not valid by IPUtils::parseRange.

Using a method of smaller progressively smaller limits on the CheckUser log, I found that the IPs causing the issues are ranges are in the DB as: X.X.00.00/X and X.X.X.00/X. Looking further at IPUtils::parseRange, it appears that it uses ip2long. When testing ip2long, it appears that it rejects IP addresses in the format X.X.X.00. However, when I remove one of the 0 characters the IP is validated.

Dreamy_Jazz claimed this task.Sep 13 2024, 10:46 AM
Dreamy_Jazz set the point value for this task to 2.
Dreamy_Jazz added a project: Trust and Safety Product Sprint (Sprint Theremin (Aug 26 - Sept. 6)).

Should be not too complicated to fix this (based on my analysis above and also that IPUtils::sanitizeIP should handle removing the leading 0s) and the problem is quite expensive for the DBs.

gerritbot added a comment.Sep 13 2024, 11:23 AM

Change #1072731 had a related patch set uploaded (by Dreamy Jazz; author: Dreamy Jazz):

[mediawiki/extensions/GlobalBlocking@master] Sanitise IP in GlobalBlockLookup::getGlobalBlockLookupConditions

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

gerritbot added a project: Patch-For-Review.Sep 13 2024, 11:23 AM
Dreamy_Jazz moved this task from Priority Backlog to Needs review on the Trust and Safety Product Sprint (Sprint Theremin (Aug 26 - Sept. 6)) board.Sep 13 2024, 11:48 AM
Dreamy_Jazz renamed this task from Expectation (readQueryRows <= 10000) by MediaWiki\Actions\ActionEntryPoint::execute not met (actual: {actualSeconds}) in trx #{trxId}:{query} to GlobalBlockLookup generates invalid IP lookup conditions that select all rows when an IPv4 contains leading 0s.Sep 13 2024, 11:51 AM
Dreamy_Jazz updated the task description. (Show Details)
Dreamy_Jazz moved this task from Untriaged to Sep 2024 on the Wikimedia-production-error board.
Dreamy_Jazz added a project: Wikimedia-Slow-DB-Query.Sep 13 2024, 11:53 AM
kostajh moved this task from Needs review to Needs QA on the Trust and Safety Product Sprint (Sprint Theremin (Aug 26 - Sept. 6)) board.Sep 13 2024, 12:02 PM
gerritbot added a comment.Sep 13 2024, 12:27 PM

Change #1072731 merged by jenkins-bot:

[mediawiki/extensions/GlobalBlocking@master] Sanitise IP in GlobalBlockLookup::getGlobalBlockLookupConditions

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

Maintenance_bot removed a project: Patch-For-Review.Sep 13 2024, 12:30 PM
ReleaseTaggerBot added a project: MW-1.43-notes (1.43.0-wmf.23; 2024-09-17).Sep 13 2024, 1:00 PM
Dreamy_Jazz moved this task from Sprint Theremin (Aug 26 - Sept. 6) to Sprint Beatboxing (Sept 16-27) on the Trust and Safety Product Sprint board.Sep 16 2024, 10:16 AM
Dreamy_Jazz edited projects, added Trust and Safety Product Sprint (Sprint Beatboxing (Sept 16-27)); removed Trust and Safety Product Sprint (Sprint Theremin (Aug 26 - Sept. 6)).
Dreamy_Jazz moved this task from Priority Backlog to Needs QA on the Trust and Safety Product Sprint (Sprint Beatboxing (Sept 16-27)) board.
Dreamy_Jazz moved this task from Sprint Beatboxing (Sept 16-27) to Sprint Cello (Oct 7 - 18) on the Trust and Safety Product Sprint board.Oct 8 2024, 10:13 AM
Dreamy_Jazz edited projects, added Trust and Safety Product Sprint (Sprint Cello (Oct 7 - 18)); removed Trust and Safety Product Sprint (Sprint Beatboxing (Sept 16-27)).
Dreamy_Jazz moved this task from Priority Backlog to Needs QA on the Trust and Safety Product Sprint (Sprint Cello (Oct 7 - 18)) board.
Dreamy_Jazz closed this task as Resolved.Oct 22 2024, 10:37 AM
Dreamy_Jazz moved this task from Needs QA to Done on the Trust and Safety Product Sprint (Sprint Cello (Oct 7 - 18)) board.

No logs are now appearing after opening the relevant URL. Given that QA doesn't have logstash access, I'm skipping QA for this.

dom_walden subscribed.Oct 22 2024, 12:43 PM

I inserted an entry to cu_log with a cul_target_text=1.2.00.00/16. Then I looked at the entry in Special:CheckUserLog. The logs showed the query GlobalBlockLookup was making is correct (as far as I can tell):

[rdbms] MediaWiki\Extension\GlobalBlocking\Services\GlobalBlockLookup::getGlobalBlockingBlock [0.214ms] mariadb-main: SELECT  gb_id,gb_address,gb_target_central_id,gb_by_central_id,gb_by_wiki,gb_reason,gb_timestamp,gb_anon_only,gb_expiry,gb_range_start,gb_range_end,gb_create_account,gb_enable_autoblock,gb_autoblock_parent_id  FROM `globalblocks`    WHERE ((gb_expiry > '20241022105107' AND (gb_range_start LIKE '0102%' ESCAPE '`' AND gb_range_start <= '01020000' AND gb_range_end >= '0102FFFF')))

(This is compared to previously where the query ended ... (gb_range_start LIKE '%' ESCAPE ''' AND gb_range_start <= 0 AND gb_range_end >= 0))))

I generated a number of IPs in various formats (including with leading zeroes), inserted them into cu_log in the same manner and inspected the log after visiting Special:CheckUserLog. I found no instance of an incorrect GlobalBlockLookup::getGlobalBlockingBlock query.

Test environment: local docker CheckUser 2.5 (236b675) 16:08, 21 October 2024. GlobalBlocking – (6d5fb35) 16:55, 21 October 2024.

Content licensed under Creative Commons Attribution-ShareAlike (CC BY-SA) 4.0 unless otherwise noted; code licensed under GNU General Public License (GPL) 2.0 or later and other open source licenses. By using this site, you agree to the Terms of Use, Privacy Policy, and Code of Conduct. · Wikimedia Foundation · Privacy Policy · Code of Conduct · Terms of Use · Disclaimer · CC-BY-SA · GPL · Credits