Last modified: 2014-08-29 19:15:37 UTC

Wikimedia Bugzilla is closed!

Wikimedia migrated from Bugzilla to Phabricator. Bug reports are handled in Wikimedia Phabricator.
This static website is read-only and for historical purposes. It is not possible to log in and except for displaying bug reports and their history, links might be broken. See T42959, the corresponding Phabricator task for complete and up-to-date bug report information.
Bug 40959 - DatabaseMysql::addIdentifierQuotes() is implemented incorrectly
DatabaseMysql::addIdentifierQuotes() is implemented incorrectly
Status: RESOLVED DUPLICATE of bug 55427
Product: MediaWiki
Classification: Unclassified
Database (Other open bugs)
unspecified
All All
: High minor (vote)
: ---
Assigned To: Nobody - You can work on this!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2012-10-11 15:09 UTC by Jared Williams
Modified: 2014-08-29 19:15 UTC (History)
8 users (show)

See Also:
Web browser: ---
Mobile Platform: ---
Assignee Huggle Beta Tester: ---


Attachments
Escape ` characters in identifiers (848 bytes, patch)
2012-10-17 00:17 UTC, Chris Steipp
Details

Description Jared Williams 2012-10-11 15:09:56 UTC
Currently DatabaseMysql::addIdentifierQuotes() uses mysql_real_escape_string() via strencode() but that is incorrect, as mysql_real_escape_string() does not escape backticks (`).

See:

http://dev.mysql.com/doc/refman/5.6/en/identifiers.html

Section that starts with "Identifier quote characters can be included within an identifier if you quote the identifier."
Comment 1 Sumana Harihareswara 2012-10-12 18:55:50 UTC
Thanks for the bug report!
Comment 2 Chris Steipp 2012-10-13 00:08:45 UTC
I need to look into this a little more, but I'm marking it a security bug for now.
Comment 3 Chris Steipp 2012-10-17 00:17:26 UTC
Created attachment 11201 [details]
Escape ` characters in identifiers

I haven't yet seen any code that passes user-controlled data into the table or field names, but this will prevent an attacks of this type, if someone ever does something like that.
Comment 4 Jared Williams 2012-10-17 14:31:22 UTC
Comment on attachment 11201 [details]
Escape ` characters in identifiers

Not sure that any escaping added by strencode()/mysql_real_escape_string() will ever be unescaped by mysql's lexer.

But given the set of characters escaped, \0, \n, \r, ", ', and \ (or just ' when mysql is running with sql_mode=NO_BACKSLASH_ESCAPES) I don't think it could cause any harm other then cause an error.
Comment 6 Chris Steipp 2013-12-18 21:13:42 UTC
This was fixed publicly in bug 55427
Comment 7 Alex Monk 2014-08-29 17:57:04 UTC
When can we get this moved out of the Security component?
Comment 8 Sam Reed (reedy) 2014-08-29 19:08:40 UTC
(In reply to Alex Monk from comment #7)
> When can we get this moved out of the Security component?

Now I guess, but I'm not sure it's so much needed as it has a public counterpart in bug 55427
Comment 9 Alex Monk 2014-08-29 19:15:37 UTC
Let's default to keeping these things public unless they actually need to be private, please.

*** This bug has been marked as a duplicate of bug 55427 ***

Note You need to log in before you can comment on or make changes to this bug.


Navigation
Links