| Daimona | |
| Nov 20 2019, 5:55 PM |
| F31112161: T238768.patch | |
| Nov 20 2019, 5:59 PM |
This was reported for itwiki filter 529. If you try to set a very short block duration (e.g. 15 minutes), no block will be issued.
I've already investigated, this happen because we pass -1 to SpecialBlock::parseExpiryInput, which in turns returns (IIUC now() + 1 hour), hence the duration provided by the "current" filter is deemed shorter and discarded.
Easy fix, coming soon.
Note: this was introduced with rEABF2dd8d27c34f8fa583ad7d9a15a5994d18062e7d5, hence we need backports for 1.31, 1.32, 1.33, and 1.34.
Update: at least on itwiki (it could be TZ-dependent), setting anything shorter than 2 hours (inclusive) doesn't work.
I'm sorry for being insistent -- currently, we're facing a tough abuser on itwiki, and short blocks would help a lot. Could someone please review the above?
@MarcoAurelio @Daimona - patch seems sane, I can deploy as a security patch sometime this afternoon after the next SWAT.
Cool, thanks! I won't be around for testing, but will provide feedback tomorrow-ish, then wait your green light for pushing backports to gerrit.
I've had a hard time testing on-wiki: the patch wasn't applied to the beta cluster, 'block' isn't enabled on testwiki, and I didn't want to block myself (again!) on itwiki. So I've tested this locally and re-confirmed it works. I also didn't see anything weird on logstash.
Hence, waiting for your green light for the backports.
I think we're good to go for the backports. I don't know if this issue necessarily requires a CVE as it seems more like a general bug in AF, which happens to be intrinsically security-sensitive. I did notice some error level warnings related to AF in logstash earlier today (around 30 or so) but I'm pretty certain they're unrelated to this patch and seem like they're probably related to issues we've seen with LegacyHandler.php in the past. e.g. on itwiki:
@timestamp: 2019-12-04T17:05:02
exception.message: PHP Warning: socket_sendto(): Host lookup failed [-10002]: Host name lookup failure
exception.trace:
#0 [internal function]: MWExceptionHandler::handleError(integer, string, string, integer, array)
#1 wmf.5/includes/debug/logger/monolog/LegacyHandler.php(217): socket_sendto(resource, string, integer, integer, string, integer)
#2 wmf.5/vendor/monolog/monolog/src/Monolog/Handler/AbstractProcessingHandler.php(39): MediaWiki\Logger\Monolog\LegacyHandler->write(array)
#3 wmf.5/vendor/monolog/monolog/src/Monolog/Handler/WhatFailureGroupHandler.php(35): Monolog\Handler\AbstractProcessingHandler->handle(array)
#4 wmf.5/vendor/monolog/monolog/src/Monolog/Logger.php(344): Monolog\Handler\WhatFailureGroupHandler->handle(array)
#5 wmf.5/vendor/monolog/monolog/src/Monolog/Logger.php(623): Monolog\Logger->addRecord(integer, string, array)
#6 wmf.5/extensions/AbuseFilter/includes/parser/AbuseFilterParser.php(1148): Monolog\Logger->debug(string)
#7 wmf.5/extensions/AbuseFilter/includes/parser/AbuseFilterCachingParser.php(127): AbuseFilterParser->getVarValue(string)
#8 wmf.5/extensions/AbuseFilter/includes/parser/AbuseFilterCachingParser.php(254): AbuseFilterCachingParser->evalNode(AFPTreeNode)
#9 wmf.5/extensions/AbuseFilter/includes/parser/AbuseFilterCachingParser.php(261): AbuseFilterCachingParser->evalNode(AFPTreeNode)
#10 wmf.5/extensions/AbuseFilter/includes/parser/AbuseFilterCachingParser.php(261): AbuseFilterCachingParser->evalNode(AFPTreeNode)
#11 wmf.5/extensions/AbuseFilter/includes/parser/AbuseFilterCachingParser.php(101): AbuseFilterCachingParser->evalNode(AFPTreeNode)
#12 wmf.5/extensions/AbuseFilter/includes/parser/AbuseFilterCachingParser.php(60): AbuseFilterCachingParser->evalTree(AFPSyntaxTree)
#13 wmf.5/extensions/AbuseFilter/includes/parser/AbuseFilterParser.php(394): AbuseFilterCachingParser->intEval(string)
#14 wmf.5/extensions/AbuseFilter/includes/AbuseFilter.php(464): AbuseFilterParser->parse(string)
#15 wmf.5/extensions/AbuseFilter/includes/AbuseFilterRunner.php(407): AbuseFilter::checkConditions(string, AbuseFilterCachingParser, boolean, string)
#16 wmf.5/extensions/AbuseFilter/includes/AbuseFilterRunner.php(340): AbuseFilterRunner->checkFilter(stdClass)
#17 wmf.5/extensions/AbuseFilter/includes/AbuseFilterRunner.php(223): AbuseFilterRunner->checkAllFilters()
#18 wmf.5/extensions/AbuseFilter/includes/AbuseFilterHooks.php(1076): AbuseFilterRunner->runForStash()
#19 wmf.5/includes/deferred/MWCallableUpdate.php(38): AbuseFilterHooks::{closure}()
#20 wmf.5/includes/deferred/DeferredUpdates.php(385): MWCallableUpdate->doUpdate()
#21 wmf.5/includes/deferred/DeferredUpdates.php(283): DeferredUpdates::attemptUpdate(MWCallableUpdate, Wikimedia\Rdbms\LBFactoryMulti)
#22 wmf.5/includes/deferred/DeferredUpdates.php(228): DeferredUpdates::run(MWCallableUpdate, Wikimedia\Rdbms\LBFactoryMulti, Monolog\Logger, BufferingStatsdDataFactory, string)
#23 wmf.5/includes/deferred/DeferredUpdates.php(146): DeferredUpdates::handleUpdateQueue(array, string, integer)
#24 wmf.5/includes/MediaWiki.php(669): DeferredUpdates::doUpdates(string, integer)
#25 wmf.5/includes/api/ApiMain.php(565): MediaWiki::preOutputCommit(DerivativeContext)
#26 wmf.5/includes/api/ApiMain.php(510): ApiMain->executeActionWithErrorHandling()
#27 wmf.5/api.php(83): ApiMain->execute()
#28 /api.php(3): require(string)
#29 {main}Thanks, then I'm going to publicly push & merge the patch above.
And yes, that error is definitely unrelated. It seems something wrong with the logging infrastructure.
Change 554578 merged by jenkins-bot:
[mediawiki/extensions/AbuseFilter@master] SECURITY: Unbreak blocks shorter than one hour
Also - feel free to block my WMF account if you need to test anything like this on-wiki :)
you would need to try to make an edit (or other abuse-filter triggering action) for the filter to block you