Page MenuHomePhabricator

Security release of AbuseFilter 1.35 throws TypeError
Closed, ResolvedPublicBUG REPORT

Description

List of steps to reproduce (step by step, including full links if applicable):

  • Go to Special:AbuseFilter/test
  • Click the Test button.

What happens?:
A TypeError is thrown.

Full stack trace:

[5dce37989ee5573d96f45fc0] /wiki/Specjalna:Filtr_nadu%C5%BCy%C4%87/test TypeError from line 322 of mwroot/extensions/AbuseFilter/includes/Views/AbuseFilterView.php: Argument 2 passed to AbuseFilterView::buildVisibilityConditions() must be an instance of PermissionManager, instance of MediaWiki\Permissions\PermissionManager given, called in mwroot/extensions/AbuseFilter/includes/Views/AbuseFilterViewTestBatch.php on line 203

Backtrace:

#0 mwroot/extensions/AbuseFilter/includes/Views/AbuseFilterViewTestBatch.php(203): AbuseFilterView->buildVisibilityConditions(Wikimedia\Rdbms\MaintainableDBConnRef, MediaWiki\Permissions\PermissionManager)
#1 mwroot/extensions/AbuseFilter/includes/Views/AbuseFilterViewTestBatch.php(152): AbuseFilterViewTestBatch->doTest()
#2 mwroot/extensions/AbuseFilter/includes/special/SpecialAbuseFilter.php(139): AbuseFilterViewTestBatch->show()
#3 mwroot/includes/specialpage/SpecialPage.php(600): SpecialAbuseFilter->execute(string)
#4 mwroot/includes/specialpage/SpecialPageFactory.php(635): SpecialPage->run(string)
#5 mwroot/includes/MediaWiki.php(307): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#6 mwroot/includes/MediaWiki.php(940): MediaWiki->performRequest()
#7 mwroot/includes/MediaWiki.php(543): MediaWiki->main()
#8 mwroot/index.php(53): MediaWiki->run()
#9 mwroot/index.php(46): wfIndexMain()
#10 {main}
}

What should have happened instead?:
A test run of a filter should have been performed.

Software version (if not a Wikimedia wiki), browser information, screenshots, other information, etc:
AbuseFilter: latest from REL1_35 branch (this commit)
MediaWiki: 1.35.2

Cause
I think the issue is just that AbuseFilterView.php is missing an import, as introduced in this commit.

Event Timeline

Followup: when the missing import is added, another exception is thrown.

[0e99669f1080f9a45114900b] /wiki/Specjalna:Filtr_nadu%C5%BCy%C4%87/test TypeError from line 1238 of mwroot/includes/Permissions/PermissionManager.php: Argument 1 passed to MediaWiki\Permissions\PermissionManager::userHasRight() must implement interface MediaWiki\User\UserIdentity, string given, called in mwroot/extensions/AbuseFilter/includes/Views/AbuseFilterView.php on line 324

Backtrace:

#0 mwroot/extensions/AbuseFilter/includes/Views/AbuseFilterView.php(324): MediaWiki\Permissions\PermissionManager->userHasRight(string)
#1 mwroot/extensions/AbuseFilter/includes/Views/AbuseFilterViewTestBatch.php(203): AbuseFilterView->buildVisibilityConditions(Wikimedia\Rdbms\MaintainableDBConnRef, MediaWiki\Permissions\PermissionManager)
#2 mwroot/extensions/AbuseFilter/includes/Views/AbuseFilterViewTestBatch.php(152): AbuseFilterViewTestBatch->doTest()
#3 mwroot/extensions/AbuseFilter/includes/special/SpecialAbuseFilter.php(139): AbuseFilterViewTestBatch->show()
#4 mwroot/includes/specialpage/SpecialPage.php(600): SpecialAbuseFilter->execute(string)
#5 mwroot/includes/specialpage/SpecialPageFactory.php(635): SpecialPage->run(string)
#6 mwroot/includes/MediaWiki.php(307): MediaWiki\SpecialPage\SpecialPageFactory->executePath(Title, RequestContext)
#7 mwroot/includes/MediaWiki.php(940): MediaWiki->performRequest()
#8 mwroot/includes/MediaWiki.php(543): MediaWiki->main()
#9 mwroot/index.php(53): MediaWiki->run()
#10 mwroot/index.php(46): wfIndexMain()
#11 {main}

Phan in LTS branches, pretty-please?

Change 682717 had a related patch set uploaded (by Reedy; author: Reedy):

[mediawiki/extensions/AbuseFilter@REL1_35] Fix backport issues

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

Change 678813 had a related patch set uploaded (by Daimona Eaytoy; author: Daimona Eaytoy):

[mediawiki/extensions/AbuseFilter@REL1_35] Fix missing import and parameter

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

Thank you Daimona for your quick response! IMO this is just case-in-point for the relevant Phan discussion and not anyone's fault :)

Just one more thing to note: I installed this only because it was announced as a supplemental security release. Do we need a follow-up to T270466, i.e. announce these fixes when they are merged?

Bongo-Cat triaged this task as Unbreak Now! priority.May 12 2021, 5:13 PM
R4356th lowered the priority of this task from Unbreak Now! to Needs Triage.May 12 2021, 7:35 PM

This does not impact WMF production and does not appear to have anyone actively working on this.

Change 678813 merged by jenkins-bot:

[mediawiki/extensions/AbuseFilter@REL1_35] Fix missing import and parameter

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

This does not impact WMF production and does not appear to have anyone actively working on this.

Please read https://www.mediawiki.org/wiki/Phabricator/Project_management#Priority_levels "Unbreak Now! – Something is broken and needs to be fixed immediately, setting anything else aside". - I don't know if the /test interface being broken meets that criteria, but "Unbreak Now!" does not have to impact WMF production, I have no idea why you'd say that..

Daimona claimed this task.
Daimona removed a project: Patch-For-Review.

Please read https://www.mediawiki.org/wiki/Phabricator/Project_management#Priority_levels "Unbreak Now! – Something is broken and needs to be fixed immediately, setting anything else aside". - I don't know if the /test interface being broken meets that criteria, but "Unbreak Now!" does not have to impact WMF production, I have no idea why you'd say that..

It's true that UBN does not have to impact WMF production but it surely helps (but obviously does not guarantee) meet the criteria. I do not think triaging a previously un-triaged task as UBN suddenly without any discussion makes sense. Anyway, thank you very much, @Daimona for fixing this!

Change 682717 abandoned by Reedy:

[mediawiki/extensions/AbuseFilter@REL1_35] Fix backport issues

Reason:

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