Page MenuHomePhabricator

Bad english MediaWiki:Abusefilter-blocker breaks filters (CVE-2021-36126)
Closed, ResolvedPublicSecurity

Description

If MediaWiki:Abusefilter-blocker is invalid in the content language, the filter user falls back to the English version, but that English version could also be invalid on a wiki - we should use Message::useDatabase() to ensure that the English version is fetched from the i18n files, rather than onwiki

Demo:
On beta metawiki, set https://meta.wikimedia.beta.wmflabs.org/wiki/MediaWiki:Abusefilter-blocker to Abuse filter #test and trigger a filter that will try to block the user
Result:

[YLsNXJQ08eKxOGcOSgAEvwAAAAA] /w/index.php?title=User:DannyS712/sandbox&action=submit TypeError: Argument 1 passed to MediaWiki\User\UserGroupManager::getUserGroups() must implement interface MediaWiki\User\UserIdentity, null given, called in /srv/mediawiki/php-master/extensions/AbuseFilter/includes/FilterUser.php on line 61

Backtrace:

from /srv/mediawiki/php-master/includes/user/UserGroupManager.php(651)
#0 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/FilterUser.php(61): MediaWiki\User\UserGroupManager->getUserGroups(NULL)
#1 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/Consequences/Consequence/BlockingConsequence.php(78): MediaWiki\Extension\AbuseFilter\FilterUser->getUser()
#2 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/Consequences/Consequence/Block.php(62): MediaWiki\Extension\AbuseFilter\Consequences\Consequence\BlockingConsequence->doBlockInternal(string, integer, string, string, boolean, boolean)
#3 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/Consequences/ConsequencesExecutor.php(317): MediaWiki\Extension\AbuseFilter\Consequences\Consequence\Block->execute()
#4 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/Consequences/ConsequencesExecutor.php(98): MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesExecutor->takeConsequenceAction(MediaWiki\Extension\AbuseFilter\Consequences\Consequence\Block)
#5 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/FilterRunner.php(261): MediaWiki\Extension\AbuseFilter\Consequences\ConsequencesExecutor->executeFilterActions(array)
#6 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/AbuseFilterHooks.php(102): MediaWiki\Extension\AbuseFilter\FilterRunner->run()
#7 /srv/mediawiki/php-master/extensions/AbuseFilter/includes/AbuseFilterHooks.php(45): MediaWiki\Extension\AbuseFilter\AbuseFilterHooks::filterEdit(DerivativeContext, User, WikitextContent, string, string)
#8 /srv/mediawiki/php-master/includes/HookContainer/HookContainer.php(338): MediaWiki\Extension\AbuseFilter\AbuseFilterHooks::onEditFilterMergedContent(DerivativeContext, WikitextContent, Status, string, User, boolean)
#9 /srv/mediawiki/php-master/includes/HookContainer/HookContainer.php(137): MediaWiki\HookContainer\HookContainer->callLegacyHook(string, array, array, array)
#10 /srv/mediawiki/php-master/includes/HookContainer/HookRunner.php(1498): MediaWiki\HookContainer\HookContainer->run(string, array)
#11 /srv/mediawiki/php-master/includes/editpage/Constraint/EditFilterMergedContentHookConstraint.php(90): MediaWiki\HookContainer\HookRunner->onEditFilterMergedContent(DerivativeContext, WikitextContent, Status, string, User, boolean)
#12 /srv/mediawiki/php-master/includes/editpage/Constraint/EditConstraintRunner.php(88): MediaWiki\EditPage\Constraint\EditFilterMergedContentHookConstraint->checkConstraint()
#13 /srv/mediawiki/php-master/includes/EditPage.php(2276): MediaWiki\EditPage\Constraint\EditConstraintRunner->checkConstraints()
#14 /srv/mediawiki/php-master/includes/EditPage.php(1695): EditPage->internalAttemptSave(NULL, boolean)
#15 /srv/mediawiki/php-master/includes/EditPage.php(670): EditPage->attemptSave(NULL)
#16 /srv/mediawiki/php-master/includes/actions/EditAction.php(71): EditPage->edit()
#17 /srv/mediawiki/php-master/includes/actions/SubmitAction.php(38): EditAction->show()
#18 /srv/mediawiki/php-master/includes/MediaWiki.php(536): SubmitAction->show()
#19 /srv/mediawiki/php-master/includes/MediaWiki.php(320): MediaWiki->performAction(Article, Title)
#20 /srv/mediawiki/php-master/includes/MediaWiki.php(917): MediaWiki->performRequest()
#21 /srv/mediawiki/php-master/includes/MediaWiki.php(551): MediaWiki->main()
#22 /srv/mediawiki/php-master/index.php(53): MediaWiki->run()
#23 /srv/mediawiki/php-master/index.php(46): wfIndexMain()
#24 /srv/mediawiki/w/index.php(3): require(string)
#25 {main}

Similar underlying cause as T53837

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript
DannyS712 added a project: Patch-For-Review.
DannyS712 added a subscriber: Daimona.


^ simple patch to just add a ->useDatabase( false ) call, @Daimona would you mind reviewing?


^ simple patch to just add a ->useDatabase( false ) call, @Daimona would you mind reviewing?

-1, line is too long and will fail PHPCS once on gerrit. Aside from that, I think the fix is OK for the short term (long term solution is to make the name non-customizable)


@Daimona I've fixed the too long line

Approved.

Thanks - there is a security window on Monday, should we plan on deploying it then? I can add a link to this task to the wikitech deployments page

sbassett triaged this task as High priority.Jun 7 2021, 3:52 PM
sbassett moved this task from Incoming to Security Patch To Deploy on the Security-Team board.

Approved.

Thanks - there is a security window on Monday, should we plan on deploying it then? I can add a link to this task to the wikitech deployments page

+1 to the patch, I can deploy it today during the security window. Would be great if someone could be around to help test the ext, otherwise I'll just keep an eye on logstash.

(since stashbot isn't here)
Mentioned in SAL (#wikimedia-operations) [2021-06-07T21:12:32Z] <sbassett> Deployed security patch for T284364

Deployed during the 2021-06-07 security window. Tested by @DannyS712. Tracking at T276237 and T279733.

sbassett lowered the priority of this task from High to Low.Jul 1 2021, 10:35 PM
sbassett changed Author Affiliation from N/A to Wikimedia Communities.
sbassett changed the visibility from "Custom Policy" to "Public (No Login Required)".
sbassett changed the edit policy from "Custom Policy" to "All Users".

Change 702765 had a related patch set uploaded (by SBassett; author: DannyS712):

[mediawiki/extensions/AbuseFilter@master] SECURITY: Avoid database for MediaWiki:Abusefilter-blocker fallback

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

Change 702721 had a related patch set uploaded (by SBassett; author: DannyS712):

[mediawiki/extensions/AbuseFilter@REL1_36] SECURITY: Avoid database for MediaWiki:Abusefilter-blocker fallback

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

Change 702765 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@master] SECURITY: Avoid database for MediaWiki:Abusefilter-blocker fallback

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

Change 702721 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@REL1_36] SECURITY: Avoid database for MediaWiki:Abusefilter-blocker fallback

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

sbassett renamed this task from Bad english MediaWiki:Abusefilter-blocker breaks filters to Bad english MediaWiki:Abusefilter-blocker breaks filters (CVE-2021-36126).Jul 2 2021, 8:03 PM
sbassett reassigned this task from sbassett to DannyS712.
sbassett moved this task from Watching to Our Part Is Done on the Security-Team board.