Page MenuHomePhabricator

[{exception_id}] {exception_url} Flow\Exception\FlowException from line 397 of /srv/mediawiki/php-1.33.0-wmf.8/extensions/Flow/includes/Block/TopicListBlock.php: The `newest` sort order does not allow the `offset` parameter. Please use `offset-id`
Open, HighPublicPRODUCTION ERROR

Description

From https://tools.wmflabs.org/versions/

2018-12-12 17:02 <zfilipin@deploy1001> Synchronized php: group1 wikis to 1.33.0-wmf.8 (duration: 00m 50s)

From https://logstash.wikimedia.org/app/kibana#/dashboard/Fatal-Monitor

[{exception_id}] {exception_url} Flow\Exception\FlowException from line 397 of /srv/mediawiki/php-1.33.0-wmf.8/extensions/Flow/includes/Block/TopicListBlock.php: The `newest` sort order does not allow the `offset` parameter. Please use `offset-id`.

Event Timeline

zeljkofilipin triaged this task as Unbreak Now! priority.Dec 12 2018, 5:26 PM
zeljkofilipin created this task.
Restricted Application changed the subtype of this task from "Release" to "Task". · View Herald Transcript
Restricted Application added subscribers: Liuxinyu970226, TerraCodes, Aklapper. · View Herald Transcript

I note the errors seem to have stopped on their own at 17:16, and that they were all on mediawikiwiki which is in group 0.

More specifically: Kibana lists the following hit totals for "The `newest` sort order does not allow the `offset` parameter":

timehits
16:45-17:070
17:083
17:0921
17:100
17:110
17:120
17:130
17:1413
17:1527
17:165
17:17 on0

That's low and bursty enough that it could well just have been a user coincidentally doing something odd.

Possibly related? [{exception_id}] {exception_url} Flow\Exception\DataModelException from line 173 of /srv/mediawiki/php-1.33.0-wmf.8/extensions/Flow/includes/Data/Index/TopKIndex.php: Unable to find specified offset in query results

could well just have been a user coincidentally doing something odd.

Oh, indeed. /w/index.php?title=Project:Support_desk&topiclist_limit=10&topiclist_offset=20181210183932&topiclist_offset-dir=fwd&topiclist_sortby=t73WaIqk'));select%20pg_sleep(10);%20--%20

greg set Security to Software security bug.Dec 12 2018, 5:49 PM
greg added a project: acl*security.
greg changed the visibility from "Public (No Login Required)" to "Custom Policy".
greg added a subscriber: greg.

Making a private security issue. Security team, please review.

Based on @Anomie's comments, this is not a train blocker. Thanks Brad.

topiclist_sortby is seen in many (all?) of these https://logstash.wikimedia.org/goto/886dfa78a08b7f99ee75a7fd0372a311 . AFAICT the value passed for that param gets passed to Title#getFullURL as the first query argument.

From a handful of URLs I just tested on mw.org (based upon T211798#4817815), it looks like the topiclist_sortby param fails for most values, when present within the query string. The drive-by sqli attempts might be a bit of a red herring in this case, as these exceptions appear to be caught and handled outside of any explicit db interaction, at least after a quick glance.

See:

  1. https://gerrit.wikimedia.org/g/mediawiki/extensions/Flow/+/b6342d86015d25d267ad8bfb9c05b800f87aaad4/includes/Block/TopicListBlock.php#397
  2. https://gerrit.wikimedia.org/g/mediawiki/extensions/Flow/+/b6342d86015d25d267ad8bfb9c05b800f87aaad4/includes/Data/Index/TopKIndex.php#173
  3. https://gerrit.wikimedia.org/g/mediawiki/extensions/Flow/+/b6342d86015d25d267ad8bfb9c05b800f87aaad4/includes/Formatter/RevisionFormatter.php#179

I've set priority to UBN since I thought it was blocking the train. Is it really UBN?

greg lowered the priority of this task from Unbreak Now! to High.Jan 3 2019, 8:06 PM
Catrope added a subscriber: Catrope.

I don't see how this is a security issue. Flow throws these exceptions for any invalid values or parameter combinations, and does not reflect the invalid value back. If the log spam is an issue, we can deal with that.

@Catrope I've noticed this in logs during train. All I care is that the log spam stops. @greg has tagged it for Security review, but I don't know if it happened.

My guess is that @greg tagged it for security because T211798#4817815 showed someone attempting SQL injection and he was being cautious in case the attempt might have succeeded. It looks like Security's answer is in T211798#4818007, which seems to have found no security problem.

@Anomie et al - that's correct, I didn't see anything here that seemed like a legitimate sqli.

greg changed the visibility from "Custom Policy" to "Public (No Login Required)".

Due to this not being user facing the Growth-Team isn't planning to work on it. However, if Operations finds it important, please reach out to let us know.

thcipriani changed the subtype of this task from "Task" to "Production Error".
thcipriani added a subscriber: thcipriani.

Still seen in 1.38.0-wmf.1:

  • 2 errors in 24 hours
  • Same user, same wiki (mediawiki.org), same server, same topic
Error
normalized_message
[{reqId}] {exception_url}   Flow\Exception\FlowException: The `updated` sort order does not allow the `offset-id` parameter. Please use `offset`.
exception.trace
from /srv/mediawiki/php-1.38.0-wmf.1/extensions/Flow/includes/Block/TopicListBlock.php(391)
#0 /srv/mediawiki/php-1.38.0-wmf.1/extensions/Flow/includes/Block/TopicListBlock.php(264): Flow\Block\TopicListBlock->getFindOptions(array)
#1 /srv/mediawiki/php-1.38.0-wmf.1/extensions/Flow/includes/View.php(235): Flow\Block\TopicListBlock->renderApi(array)
#2 /srv/mediawiki/php-1.38.0-wmf.1/extensions/Flow/includes/View.php(71): Flow\View->buildApiResponse(Flow\WorkflowLoader, array, string, array)
#3 /srv/mediawiki/php-1.38.0-wmf.1/extensions/Flow/includes/Actions/FlowAction.php(107): Flow\View->show(Flow\WorkflowLoader, string)
#4 /srv/mediawiki/php-1.38.0-wmf.1/extensions/Flow/includes/Actions/ViewAction.php(27): Flow\Actions\FlowAction->showForAction(string, OutputPage)
#5 /srv/mediawiki/php-1.38.0-wmf.1/extensions/Flow/includes/Actions/FlowAction.php(50): Flow\Actions\ViewAction->showForAction(string)
#6 /srv/mediawiki/php-1.38.0-wmf.1/includes/MediaWiki.php(538): Flow\Actions\FlowAction->show()
#7 /srv/mediawiki/php-1.38.0-wmf.1/includes/MediaWiki.php(320): MediaWiki->performAction(Article, Title)
#8 /srv/mediawiki/php-1.38.0-wmf.1/includes/MediaWiki.php(925): MediaWiki->performRequest()
#9 /srv/mediawiki/php-1.38.0-wmf.1/includes/MediaWiki.php(559): MediaWiki->main()
#10 /srv/mediawiki/php-1.38.0-wmf.1/index.php(53): MediaWiki->run()
#11 /srv/mediawiki/php-1.38.0-wmf.1/index.php(46): wfIndexMain()
#12 /srv/mediawiki/w/index.php(3): require(string)
#13 {main}

Still seen in 1.38.0-wmf.23 and 24:

  • 84 errors in 24 hours
  • All from frwiki
Error
normalized_message
[{reqId}] {exception_url}   Flow\Exception\FlowException: The `newest` sort order does not allow the `offset` parameter.  Please use `offset-id`.
exception.trace
from /srv/mediawiki/php-1.38.0-wmf.24/extensions/Flow/includes/Block/TopicListBlock.php(406)
#0 /srv/mediawiki/php-1.38.0-wmf.24/extensions/Flow/includes/Block/TopicListBlock.php(263): Flow\Block\TopicListBlock->getFindOptions(array)
#1 /srv/mediawiki/php-1.38.0-wmf.24/extensions/Flow/includes/View.php(233): Flow\Block\TopicListBlock->renderApi(array)
#2 /srv/mediawiki/php-1.38.0-wmf.24/extensions/Flow/includes/View.php(71): Flow\View->buildApiResponse(Flow\WorkflowLoader, array, string, array)
#3 /srv/mediawiki/php-1.38.0-wmf.24/extensions/Flow/includes/Actions/FlowAction.php(107): Flow\View->show(Flow\WorkflowLoader, string)
#4 /srv/mediawiki/php-1.38.0-wmf.24/extensions/Flow/includes/Actions/ViewAction.php(27): Flow\Actions\FlowAction->showForAction(string, OutputPage)
#5 /srv/mediawiki/php-1.38.0-wmf.24/extensions/Flow/includes/Actions/FlowAction.php(50): Flow\Actions\ViewAction->showForAction(string)
#6 /srv/mediawiki/php-1.38.0-wmf.24/includes/MediaWiki.php(544): Flow\Actions\FlowAction->show()
#7 /srv/mediawiki/php-1.38.0-wmf.24/includes/MediaWiki.php(321): MediaWiki->performAction(Article, Title)
#8 /srv/mediawiki/php-1.38.0-wmf.24/includes/MediaWiki.php(910): MediaWiki->performRequest()
#9 /srv/mediawiki/php-1.38.0-wmf.24/includes/MediaWiki.php(564): MediaWiki->main()
#10 /srv/mediawiki/php-1.38.0-wmf.24/index.php(53): MediaWiki->run()
#11 /srv/mediawiki/php-1.38.0-wmf.24/index.php(46): wfIndexMain()
#12 /srv/mediawiki/w/index.php(3): require(string)
#13 {main}
brennen added a subscriber: brennen.

Seen in 1.39.0-wmf.22:

Error
normalized_message
[{reqId}] {exception_url}   Flow\Exception\FlowException: The `updated` sort order does not allow the `offset-id` parameter. Please use `offset`.
exception.trace
from /srv/mediawiki/php-1.39.0-wmf.22/extensions/Flow/includes/Block/TopicListBlock.php(393)
#0 /srv/mediawiki/php-1.39.0-wmf.22/extensions/Flow/includes/Block/TopicListBlock.php(263): Flow\Block\TopicListBlock->getFindOptions(array)
#1 /srv/mediawiki/php-1.39.0-wmf.22/extensions/Flow/includes/View.php(233): Flow\Block\TopicListBlock->renderApi(array)
#2 /srv/mediawiki/php-1.39.0-wmf.22/extensions/Flow/includes/View.php(71): Flow\View->buildApiResponse(Flow\WorkflowLoader, array, string, array)
#3 /srv/mediawiki/php-1.39.0-wmf.22/extensions/Flow/includes/Actions/FlowAction.php(107): Flow\View->show(Flow\WorkflowLoader, string)
#4 /srv/mediawiki/php-1.39.0-wmf.22/extensions/Flow/includes/Actions/ViewAction.php(27): Flow\Actions\FlowAction->showForAction(string, OutputPage)
#5 /srv/mediawiki/php-1.39.0-wmf.22/extensions/Flow/includes/Actions/FlowAction.php(50): Flow\Actions\ViewAction->showForAction(string)
#6 /srv/mediawiki/php-1.39.0-wmf.22/includes/MediaWiki.php(548): Flow\Actions\FlowAction->show()
#7 /srv/mediawiki/php-1.39.0-wmf.22/includes/MediaWiki.php(322): MediaWiki->performAction(Article, Title)
#8 /srv/mediawiki/php-1.39.0-wmf.22/includes/MediaWiki.php(911): MediaWiki->performRequest()
#9 /srv/mediawiki/php-1.39.0-wmf.22/includes/MediaWiki.php(568): MediaWiki->main()
#10 /srv/mediawiki/php-1.39.0-wmf.22/index.php(50): MediaWiki->run()
#11 /srv/mediawiki/php-1.39.0-wmf.22/index.php(46): wfIndexMain()
#12 /srv/mediawiki/w/index.php(3): require(string)
#13 {main}