Page MenuHomePhabricator
Create Task
Maniphest T284364

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

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

Details

Author Affiliation
Wikimedia Communities
ProjectBranchLines +/-Subject
mediawiki/extensions/AbuseFiltermaster+5 -1SECURITY: Avoid database for MediaWiki:Abusefilter-blocker fallback
mediawiki/extensions/AbuseFilterREL1_36+5 -1SECURITY: Avoid database for MediaWiki:Abusefilter-blocker fallback
Customize query in gerrit

Related Objects

Event Timeline

DannyS712 created this task.Jun 5 2021, 6:22 AM
Restricted Application added a project: User-DannyS712. · View Herald TranscriptJun 5 2021, 6:22 AM
Restricted Application added a subscriber: Aklapper. · View Herald Transcript
DannyS712 added a project: AbuseFilter.Jun 5 2021, 6:23 AM
DannyS712 moved this task from Unsorted to Awaiting review and deployment on the User-DannyS712 board.Jun 5 2021, 6:28 AM
DannyS712 added a project: Patch-For-Review.
DannyS712 added a subscriber: Daimona.

T284364.patch1 KBDownload

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

DannyS712 updated the task description. (Show Details)Jun 5 2021, 6:29 AM
Daimona added a comment.Jun 5 2021, 1:37 PM
In T284364#7135981, @DannyS712 wrote:

T284364.patch1 KBDownload

^ 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)

MarcoAurelio added a subscriber: MarcoAurelio.Jun 6 2021, 11:51 AM

T160666 might be of interest.

DannyS712 added a comment.EditedJun 6 2021, 12:00 PM

T284364-2.patch1 KBDownload

@Daimona I've fixed the too long line

Daimona added a comment.Jun 6 2021, 4:37 PM

Approved.

DannyS712 added a comment.Jun 6 2021, 4:51 PM
In T284364#7136885, @Daimona wrote:

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.
sbassett added a subscriber: sbassett.Jun 7 2021, 4:44 PM
In T284364#7136905, @DannyS712 wrote:
In T284364#7136885, @Daimona wrote:

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.

sbassett claimed this task.Jun 7 2021, 4:44 PM
sbassett mentioned this in T279733: Write and send supplementary release announcement for extensions and skins with security patches (1.31.15/1.35.3/1.36.1).Jun 7 2021, 9:14 PM
DannyS712 removed a project: Patch-For-Review.Jun 7 2021, 9:16 PM

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

sbassett added a comment.EditedJun 7 2021, 9:17 PM

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

sbassett moved this task from Security Patch To Deploy to Watching on the Security-Team board.Jun 29 2021, 3:28 PM
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".
gerritbot added a comment.Jul 1 2021, 10:36 PM

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

gerritbot added a project: Patch-For-Review.Jul 1 2021, 10:36 PM

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

gerritbot added a comment.Jul 2 2021, 3:46 PM

Change 702765 merged by jenkins-bot:

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

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

ReleaseTaggerBot added a project: MW-1.37-notes (1.37.0-wmf.14; 2021-07-12).Jul 2 2021, 4:00 PM
gerritbot added a comment.Jul 2 2021, 4:10 PM

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 closed this task as Resolved.Jul 2 2021, 8:14 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.
Content licensed under Creative Commons Attribution-ShareAlike 3.0 (CC-BY-SA) unless otherwise noted; code licensed under GNU General Public License (GPL) or 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