r50683 MediaWiki - Code Review archive

Repository:MediaWiki
Revision:r50682‎ | r50683 | r50684 >
Date:12:25, 17 May 2009
Author:ashley
Status:deferred (Comments)
Tags:
Comment:
FCKeditor: reverting one SQL change back to raw SQL - was getting database errors
Modified paths:
  • /trunk/extensions/FCKeditor/FCKeditorSajax.body.php (modified) (history)

Diff [purge]

Index: trunk/extensions/FCKeditor/FCKeditorSajax.body.php
@@ -198,15 +198,13 @@
199199 $limit = 50;
200200 $ns = NS_CATEGORY;
201201 $dbr = wfGetDB( DB_SLAVE );
202 - $res = $dbr->select(
203 - array( 'categorylinks', 'page' ),
204 - array( 'page_title AS title' ),
205 - array( 'cl_to LIKE ' . $dbr->escapeLike( $m_root ), "page_namespace = $ns" ),
206 - __METHOD__,
207 - array(),
208 - array( 'page' => array( 'LEFT JOIN', 'cl_from = page_id' ) )
209 - );
 202+ /// @todo FIXME: should use Database class
 203+ $m_root = str_replace( "'", "\'", $m_root );
 204+ $sql = "SELECT tmpSelectCatPage.page_title AS title FROM ".$dbr->tableName('categorylinks')." AS tmpSelectCat ".
 205+ "LEFT JOIN ".$dbr->tableName('page')." AS tmpSelectCatPage ON tmpSelectCat.cl_from = tmpSelectCatPage.page_id ".
 206+ "WHERE tmpSelectCat.cl_to LIKE '$m_root' AND tmpSelectCatPage.page_namespace = $ns";
210207
 208+ $res = $dbr->query( $sql, __METHOD__ );
211209 $ret = '';
212210 $i = 0;
213211 while ( ( $row = $dbr->fetchObject( $res ) ) ) {

Follow-up revisions

RevisionCommit summaryAuthorDate
r50709Fix SQL injection in FCKeditor caused by r50683...simetrical00:23, 18 May 2009

Comments

#Comment by Simetrical (talk | contribs)   00:25, 18 May 2009

This is unsafe. You need to use $dbr->addQuotes(). Consider what would happen if $m_root were a string like

\' OR 1 --

That might not quite work in MySQL, but variants might, or it might work in other database engines. I've fixed it in r50709, hopefully not breaking anything else in the process.

Status & tagging log