Page MenuHomePhabricator

ApiQueryContributions::execute "You have an error in your SQL syntax" rev_user_text
Closed, ResolvedPublic

Description

I think this is a core api syntax error, if not, please route adequately:

ApiQueryContributions::execute	10.64.48.173	1064	You have an error in your SQL syntax; check the manual that corresponds to your MariaDB server version for the right syntax to use near 'YY.YY.YY.90)  ORDER BY rev_user_text  LIMIT 501' at line 1 (10.64.48.173)	SELECT DISTINCT NULL AS `actor_id`,rev_user AS `user_id`,rev_user_text AS `user_name`  FROM `revision`    WHERE (rev_user_text LIKE 'XX.XX.X%' ESCAPE '`' ) AND (rev_user_text YY.YY.YY.90)  ORDER BY rev_user_text  LIMIT 501

The errors are not that many, but this seems to me like a serious one with an easy fix:

https://logstash.wikimedia.org/goto/866e7cb237c59480b06b906c678949cf

Event Timeline

jcrespo created this task.Mar 23 2018, 11:54 AM
Restricted Application added a subscriber: Aklapper. · View Herald TranscriptMar 23 2018, 11:54 AM
Anomie set Security to Software security bug.Mar 23 2018, 1:18 PM
Anomie added a project: Security.
Anomie changed the visibility from "Public (No Login Required)" to "Custom Policy".

Paranoia, in case this is an SQL injection vector.

It seems to start on the 1st March, and affect contributORS, too https://logstash.wikimedia.org/goto/2b74671e257bd3d848ad1956eb8cd219

It seems to start on the 1st March, and affect contributORS, too https://logstash.wikimedia.org/goto/2b74671e257bd3d848ad1956eb8cd219

That one is from T188813: internal_api_error_DBQueryError (API): Undefined variable: actorQuery, which should already be fixed.

Anomie triaged this task as Unbreak Now! priority.Mar 23 2018, 1:40 PM

Let's get this one fixed ASAP. I haven't tried it, but I think it'll be an injection if you can create a malicious username that is the 501st, 1001st, etc name for some combination of ucuserprefix, ucdir, and uccontinue, and the 500 (or 1000, etc) users before your username have few enough contributions that processing of the query gets to your username.

This should do it:

Anomie moved this task from Unsorted to Needs Review on the MediaWiki-API board.Mar 23 2018, 1:45 PM

Let's get this one fixed ASAP. I haven't tried it, but I think it'll be an injection if you can create a malicious username that is the 501st, 1001st, etc name for some combination of ucuserprefix, ucdir, and uccontinue, and the 500 (or 1000, etc) users before your username have few enough contributions that processing of the query gets to your username.
This should do it:

+1. Yep looks like that fixes the issue. I agree this is an exploitable SQLi

I'd kind of prefer that instead of $from the variable was named something like $fromSQLSnippet so that its very clear the variable contains SQL, but that's not important at the moment.

[15:02] logmsgbot !log bawolff@tin Synchronized php-1.31.0-wmf.26/includes/api/ApiQueryUserContributions.php: T190507 (duration: 00m 59s)

Bawolff changed the visibility from "Custom Policy" to "Public (No Login Required)".Mar 23 2018, 3:08 PM
Restricted Application added subscribers: Liuxinyu970226, TerraCodes. · View Herald TranscriptMar 23 2018, 3:08 PM

Change 421544 had a related patch set uploaded (by Brian Wolff; owner: Anomie):
[mediawiki/core@master] SECURITY: Fix variable usage in ApiQueryUserContributions

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

Bawolff closed this task as Resolved.Mar 23 2018, 3:09 PM
Bawolff claimed this task.

Change 421544 merged by jenkins-bot:
[mediawiki/core@master] SECURITY: Fix variable usage in ApiQueryUserContributions

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

mmodell changed the subtype of this task from "Task" to "Production Error".Aug 28 2019, 11:09 PM