Page MenuHomePhabricator

Wiki admins can break Special:Homepage by bypassing validation logic
Closed, ResolvedPublic

Description

If an admin manages to add invalid task handler ID to MediaWiki:NewcomerTasks.json, the homepage will fatal:

[053d469a97257bd3fbb38cbc] /mw/index.php?title=Special:Homepage&source=personaltoolslink&namespace=8 OutOfBoundsException: No task type handler registered for the ID foo

Backtrace:

from /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/NewcomerTasks/TaskType/TaskTypeHandlerRegistry.php(103)
#0 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/NewcomerTasks/TaskType/TaskTypeHandlerRegistry.php(48): GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry->createHandler(string)
#1 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/Config/Validation/NewcomerTasksValidator.php(42): GrowthExperiments\NewcomerTasks\TaskType\TaskTypeHandlerRegistry->get(string)
#2 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/Config/WikiPageConfigLoader.php(165): GrowthExperiments\Config\Validation\NewcomerTasksValidator->validate(array)
#3 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/Config/WikiPageConfigLoader.php(130): GrowthExperiments\Config\WikiPageConfigLoader->loadUncached(Title, integer)
#4 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/NewcomerTasks/ConfigurationLoader/PageConfigurationLoader.php(131): GrowthExperiments\Config\WikiPageConfigLoader->load(Title)
#5 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/NewcomerTasks/TaskSuggester/RemoteSearchTaskSuggesterFactory.php(64): GrowthExperiments\NewcomerTasks\ConfigurationLoader\PageConfigurationLoader->loadTaskTypes()
#6 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/Homepage/HomepageModuleRegistry.php(113): GrowthExperiments\NewcomerTasks\TaskSuggester\RemoteSearchTaskSuggesterFactory->create()
#7 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/Homepage/HomepageModuleRegistry.php(55): GrowthExperiments\Homepage\HomepageModuleRegistry::GrowthExperiments\Homepage\{closure}(MediaWiki\MediaWikiServices, RequestContext)
#8 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/Specials/SpecialHomepage.php(223): GrowthExperiments\Homepage\HomepageModuleRegistry->get(string, RequestContext)
#9 /home/urbanecm/unsynced/gerrit/mediawiki/extensions/GrowthExperiments/includes/Specials/SpecialHomepage.php(125): GrowthExperiments\Specials\SpecialHomepage->getModules(boolean, NULL)
#10 /home/urbanecm/unsynced/gerrit/mediawiki/core/includes/specialpage/SpecialPage.php(646): GrowthExperiments\Specials\SpecialHomepage->execute(NULL)
#11 /home/urbanecm/unsynced/gerrit/mediawiki/core/includes/specialpage/SpecialPageFactory.php(1366): SpecialPage->run(NULL)
#12 /home/urbanecm/unsynced/gerrit/mediawiki/core/includes/MediaWiki.php(314): MediaWiki\SpecialPage\SpecialPageFactory->executePath(string, RequestContext)
#13 /home/urbanecm/unsynced/gerrit/mediawiki/core/includes/MediaWiki.php(925): MediaWiki->performRequest()
#14 /home/urbanecm/unsynced/gerrit/mediawiki/core/includes/MediaWiki.php(559): MediaWiki->main()
#15 /home/urbanecm/unsynced/gerrit/mediawiki/core/index.php(53): MediaWiki->run()
#16 /home/urbanecm/unsynced/gerrit/mediawiki/core/index.php(46): wfIndexMain()
#17 {main}

This can be prevented by changing validation logic to catch OutOfBoundsException and return a Status object in that case. If that's done, it will still be possible to save invalid validation, but reading invalid config will gracefully fail instead of fataling.

Notes

I'm intentionally filling this public. Because we trust on wiki admins enough to disrupt newcomer's behavior by exposing Special:EditGrowthConfig (turning most modules off is possible, and that'd have similar effect as a fatal error in terms of retention/activation impact by the homepage), this is of very little Security impact.

Event Timeline

Restricted Application added a subscriber: Aklapper. · View Herald Transcript

Change 715229 had a related patch set uploaded (by Urbanecm; author: Urbanecm):

[mediawiki/extensions/GrowthExperiments@master] NewcomerTasksValidator: Catch OutOfBoundsException properly

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

Change 715229 merged by jenkins-bot:

[mediawiki/extensions/GrowthExperiments@master] NewcomerTasksValidator: Catch invalid task handler IDs properly

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