Page MenuHomePhabricator

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

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

Anomie set Security to Software security bug.Mar 23 2018, 1:18 PM
Anomie added a project: acl*security.
Anomie changed the visibility from "Public (No Login Required)" to "Custom Policy".

Paranoia, in case this is an SQL injection vector.

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:

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

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 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