Skip to content
This repository has been archived by the owner on Nov 3, 2023. It is now read-only.

[RTM] Quote reserved words in database queries #8813

Merged
merged 5 commits into from Jan 19, 2018
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 17 additions & 17 deletions system/modules/core/drivers/DC_Table.php
Expand Up @@ -466,7 +466,7 @@ public function show()

foreach ((array) $value as $v)
{
$objKey = $this->Database->prepare("SELECT " . $chunks[1] . " AS value FROM " . $chunks[0] . " WHERE id=?")
$objKey = $this->Database->prepare("SELECT " . \Database::quoteIdentifier($chunks[1]) . " AS value FROM " . $chunks[0] . " WHERE id=?")
->limit(1)
->execute($v);

Expand Down Expand Up @@ -2986,12 +2986,12 @@ protected function save($varValue)
{
if ($GLOBALS['TL_DCA'][$this->strTable]['list']['sorting']['mode'] == 4)
{
$this->Database->prepare("UPDATE " . $this->strTable . " SET " . $this->strField . "='' WHERE pid=?")
$this->Database->prepare("UPDATE " . $this->strTable . " SET " . \Database::quoteIdentifier($this->strField) . "='' WHERE pid=?")
->execute($this->activeRecord->pid);
}
else
{
$this->Database->execute("UPDATE " . $this->strTable . " SET " . $this->strField . "=''");
$this->Database->execute("UPDATE " . $this->strTable . " SET " . \Database::quoteIdentifier($this->strField) . "=''");
}
}

Expand All @@ -3004,7 +3004,7 @@ protected function save($varValue)
$arrValues = $this->values;
array_unshift($arrValues, $varValue);

$objUpdateStmt = $this->Database->prepare("UPDATE " . $this->strTable . " SET " . $this->strField . "=? WHERE " . implode(' AND ', $this->procedure))
$objUpdateStmt = $this->Database->prepare("UPDATE " . $this->strTable . " SET " . \Database::quoteIdentifier($this->strField) . "=? WHERE " . implode(' AND ', $this->procedure))
->execute($arrValues);

if ($objUpdateStmt->affectedRows)
Expand Down Expand Up @@ -3349,12 +3349,12 @@ protected function treeView()
{
list($t, $f) = explode('.', $GLOBALS['TL_DCA'][$this->strTable]['fields'][$fld]['foreignKey']);

$objRoot = $this->Database->prepare("SELECT $for FROM {$this->strTable} WHERE (" . sprintf($strPattern, $fld) . " OR " . sprintf($strPattern, "(SELECT $f FROM $t WHERE $t.id={$this->strTable}.$fld)") . ") GROUP BY $for")
$objRoot = $this->Database->prepare("SELECT $for FROM {$this->strTable} WHERE (" . sprintf($strPattern, \Database::quoteIdentifier($fld)) . " OR " . sprintf($strPattern, "(SELECT ".\Database::quoteIdentifier($f)." FROM $t WHERE $t.id={$this->strTable}.".\Database::quoteIdentifier($fld).")") . ") GROUP BY $for")
->execute($session['search'][$this->strTable]['value'], $session['search'][$this->strTable]['value']);
}
else
{
$objRoot = $this->Database->prepare("SELECT $for FROM {$this->strTable} WHERE " . sprintf($strPattern, $fld) . " GROUP BY $for")
$objRoot = $this->Database->prepare("SELECT $for FROM {$this->strTable} WHERE " . sprintf($strPattern, \Database::quoteIdentifier($fld)) . " GROUP BY $for")
->execute($session['search'][$this->strTable]['value']);
}
}
Expand Down Expand Up @@ -3692,7 +3692,7 @@ protected function generateTree($table, $id, $arrPrevNext, $blnHasSorting, $intM
list($strKey, $strTable) = explode(':', $v);
list($strTable, $strField) = explode('.', $strTable);

$objRef = $this->Database->prepare("SELECT " . $strField . " FROM " . $strTable . " WHERE id=?")
$objRef = $this->Database->prepare("SELECT " . \Database::quoteIdentifier($strField) . " FROM " . $strTable . " WHERE id=?")
->limit(1)
->execute($objRow->$strKey);

Expand Down Expand Up @@ -3989,7 +3989,7 @@ protected function parentView()
{
$arrForeignKey = explode('.', $GLOBALS['TL_DCA'][$this->ptable]['fields'][$v]['foreignKey'], 2);

$objLabel = $this->Database->prepare("SELECT " . $arrForeignKey[1] . " AS value FROM " . $arrForeignKey[0] . " WHERE id=?")
$objLabel = $this->Database->prepare("SELECT " . \Database::quoteIdentifier($arrForeignKey[1]) . " AS value FROM " . $arrForeignKey[0] . " WHERE id=?")
->limit(1)
->execute($_v);

Expand Down Expand Up @@ -4095,7 +4095,7 @@ protected function parentView()
if (isset($GLOBALS['TL_DCA'][$this->strTable]['fields'][$firstOrderBy]['foreignKey']))
{
$key = explode('.', $GLOBALS['TL_DCA'][$this->strTable]['fields'][$firstOrderBy]['foreignKey'], 2);
$query = "SELECT *, (SELECT ". $key[1] ." FROM ". $key[0] ." WHERE ". $this->strTable .".". $firstOrderBy ."=". $key[0] .".id) AS foreignKey FROM " . $this->strTable;
$query = "SELECT *, (SELECT ". \Database::quoteIdentifier($key[1]) ." FROM ". $key[0] ." WHERE ". $this->strTable .".". $firstOrderBy ."=". $key[0] .".id) AS foreignKey FROM " . $this->strTable;
$orderBy[0] = 'foreignKey';
}
}
Expand Down Expand Up @@ -4448,7 +4448,7 @@ protected function listView()
$firstOrderBy = 'pid';
$showFields = $GLOBALS['TL_DCA'][$table]['list']['label']['fields'];

$query .= " ORDER BY (SELECT " . $showFields[0] . " FROM " . $this->ptable . " WHERE " . $this->ptable . ".id=" . $this->strTable . ".pid), " . implode(', ', $orderBy);
$query .= " ORDER BY (SELECT " . \Database::quoteIdentifier($showFields[0]) . " FROM " . $this->ptable . " WHERE " . $this->ptable . ".id=" . $this->strTable . ".pid), " . implode(', ', $orderBy);

// Set the foreignKey so that the label is translated (also for backwards compatibility)
if ($GLOBALS['TL_DCA'][$table]['fields']['pid']['foreignKey'] == '')
Expand Down Expand Up @@ -4593,7 +4593,7 @@ protected function listView()
list($strKey, $strTable) = explode(':', $v);
list($strTable, $strField) = explode('.', $strTable);

$objRef = $this->Database->prepare("SELECT " . $strField . " FROM " . $strTable . " WHERE id=?")
$objRef = $this->Database->prepare("SELECT " . \Database::quoteIdentifier($strField) . " FROM " . $strTable . " WHERE id=?")
->limit(1)
->execute($row[$strKey]);

Expand Down Expand Up @@ -4957,7 +4957,7 @@ protected function searchMenu()
{
try
{
$this->Database->prepare("SELECT * FROM " . $this->strTable . " WHERE " . $strField . " REGEXP ?")
$this->Database->prepare("SELECT * FROM " . $this->strTable . " WHERE " . \Database::quoteIdentifier($strField) . " REGEXP ?")
->limit(1)
->execute($strKeyword);
}
Expand Down Expand Up @@ -4988,12 +4988,12 @@ protected function searchMenu()
if (isset($GLOBALS['TL_DCA'][$this->strTable]['fields'][$fld]['foreignKey']))
{
list($t, $f) = explode('.', $GLOBALS['TL_DCA'][$this->strTable]['fields'][$fld]['foreignKey']);
$this->procedure[] = "(" . sprintf($strPattern, $fld) . " OR " . sprintf($strPattern, "(SELECT $f FROM $t WHERE $t.id={$this->strTable}.$fld)") . ")";
$this->procedure[] = "(" . sprintf($strPattern, \Database::quoteIdentifier($fld)) . " OR " . sprintf($strPattern, "(SELECT ".\Database::quoteIdentifier($f)." FROM $t WHERE $t.id={$this->strTable}.".\Database::quoteIdentifier($fld).")") . ")";
$this->values[] = $session['search'][$this->strTable]['value'];
}
else
{
$this->procedure[] = sprintf($strPattern, $fld);
$this->procedure[] = sprintf($strPattern, \Database::quoteIdentifier($fld));
}

$this->values[] = $session['search'][$this->strTable]['value'];
Expand Down Expand Up @@ -5592,7 +5592,7 @@ protected function filterMenu($intFilterPanel)
{
$key = explode('.', $GLOBALS['TL_DCA'][$this->strTable]['fields'][$field]['foreignKey'], 2);

$objParent = $this->Database->prepare("SELECT " . $key[1] . " AS value FROM " . $key[0] . " WHERE id=?")
$objParent = $this->Database->prepare("SELECT " . \Database::quoteIdentifier($key[1]) . " AS value FROM " . $key[0] . " WHERE id=?")
->limit(1)
->execute($vv);

Expand All @@ -5619,7 +5619,7 @@ protected function filterMenu($intFilterPanel)
$showFields[0] = 'id';
}

$objShowFields = $this->Database->prepare("SELECT " . $showFields[0] . " FROM ". $this->ptable . " WHERE id=?")
$objShowFields = $this->Database->prepare("SELECT " . \Database::quoteIdentifier($showFields[0]) . " FROM ". $this->ptable . " WHERE id=?")
->limit(1)
->execute($vv);

Expand Down Expand Up @@ -5743,7 +5743,7 @@ protected function formatCurrentValue($field, $value, $mode)
{
$key = explode('.', $GLOBALS['TL_DCA'][$this->strTable]['fields'][$field]['foreignKey'], 2);

$objParent = $this->Database->prepare("SELECT " . $key[1] . " AS value FROM " . $key[0] . " WHERE id=?")
$objParent = $this->Database->prepare("SELECT " . \Database::quoteIdentifier($key[1]) . " AS value FROM " . $key[0] . " WHERE id=?")
->limit(1)
->execute($value);

Expand Down
20 changes: 19 additions & 1 deletion system/modules/core/library/Contao/Database.php
Expand Up @@ -396,7 +396,7 @@ public function getFieldNames($strTable, $blnNoCache=false)
*/
public function isUniqueValue($strTable, $strField, $varValue, $intId=null)
{
$strQuery = "SELECT * FROM $strTable WHERE $strField=?";
$strQuery = "SELECT * FROM $strTable WHERE ".static::quoteIdentifier($strField)."=?";

if ($intId !== null)
{
Expand Down Expand Up @@ -593,6 +593,24 @@ public function getUuid()
}


/**
* Quote an identifier if it is a reserved word
*
* @param string $strName
*
* @return string
*/
public static function quoteIdentifier($strName)
{
if (strtolower($strName) == 'rows')
{
$strName = '`'.$strName.'`';
}

return $strName;
}


/**
* Connect to the database server and select the database
*/
Expand Down
5 changes: 5 additions & 0 deletions system/modules/core/library/Contao/Database/Mysql.php
Expand Up @@ -95,6 +95,11 @@ protected function get_error()
*/
protected function find_in_set($strKey, $varSet, $blnIsField=false)
{
if (preg_match('/^[A-Za-z0-9_$]+$/', $strKey))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This check is not necessary as it is handled by quoteIdentifier() itself.

{
$strKey = static::quoteIdentifier($strKey);
}

if ($blnIsField)
{
return "FIND_IN_SET(" . $strKey . ", " . $varSet . ")";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In this case we should probably use "FIND_IN_SET(" . $strKey . ", " . static::quoteIdentifier($varSet) . ")"; because $varSet is then an identifier too.

Expand Down
5 changes: 5 additions & 0 deletions system/modules/core/library/Contao/Database/Mysqli.php
Expand Up @@ -85,6 +85,11 @@ protected function get_error()
*/
protected function find_in_set($strKey, $varSet, $blnIsField=false)
{
if (preg_match('/^[A-Za-z0-9_$]+$/', $strKey))
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

{
$strKey = static::quoteIdentifier($strKey);
}

if ($blnIsField)
{
return "FIND_IN_SET(" . $strKey . ", " . $varSet . ")";
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here.

Expand Down
4 changes: 2 additions & 2 deletions system/modules/core/library/Contao/Database/Statement.php
Expand Up @@ -197,7 +197,7 @@ public function set($arrParams)
if (strncasecmp($this->strQuery, 'INSERT', 6) === 0)
{
$strQuery = sprintf('(%s) VALUES (%s)',
implode(', ', array_keys($arrParams)),
implode(', ', array_map('Database::quoteIdentifier', array_keys($arrParams))),
str_replace('%', '%%', implode(', ', array_values($arrParams))));
}
// UPDATE
Expand All @@ -207,7 +207,7 @@ public function set($arrParams)

foreach ($arrParams as $k=>$v)
{
$arrSet[] = $k . '=' . $v;
$arrSet[] = \Database::quoteIdentifier($k) . '=' . $v;
}

$strQuery = 'SET ' . str_replace('%', '%%', implode(', ', $arrSet));
Expand Down
8 changes: 4 additions & 4 deletions system/modules/core/library/Contao/Model.php
Expand Up @@ -470,7 +470,7 @@ public function save()
}

// Update the row
$objDatabase->prepare("UPDATE " . static::$strTable . " %s WHERE " . static::$strPk . "=?")
$objDatabase->prepare("UPDATE " . static::$strTable . " %s WHERE " . \Database::quoteIdentifier(static::$strPk) . "=?")
->set($arrSet)
->execute($intPk);

Expand Down Expand Up @@ -563,7 +563,7 @@ public function delete()
}

// Delete the row
$intAffected = \Database::getInstance()->prepare("DELETE FROM " . static::$strTable . " WHERE " . static::$strPk . "=?")
$intAffected = \Database::getInstance()->prepare("DELETE FROM " . static::$strTable . " WHERE " . \Database::quoteIdentifier(static::$strPk) . "=?")
->execute($intPk)
->affectedRows;

Expand Down Expand Up @@ -624,7 +624,7 @@ public function getRelated($strKey, array $arrOptions=array())
elseif ($arrRelation['type'] == 'hasMany' || $arrRelation['type'] == 'belongsToMany')
{
$arrValues = deserialize($this->$strKey, true);
$strField = $arrRelation['table'] . '.' . $arrRelation['field'];
$strField = $arrRelation['table'] . '.' . \Database::quoteIdentifier($arrRelation['field']);

// Handle UUIDs (see #6525)
if ($strField == 'tl_files.uuid')
Expand Down Expand Up @@ -668,7 +668,7 @@ public function refresh()
}

// Reload the database record
$res = \Database::getInstance()->prepare("SELECT * FROM " . static::$strTable . " WHERE " . static::$strPk . "=?")
$res = \Database::getInstance()->prepare("SELECT * FROM " . static::$strTable . " WHERE " . \Database::quoteIdentifier(static::$strPk) . "=?")
->execute($intPk);

$this->setRow($res->row());
Expand Down
8 changes: 4 additions & 4 deletions system/modules/core/library/Contao/Model/QueryBuilder.php
Expand Up @@ -53,10 +53,10 @@ public static function find(array $arrOptions)

foreach (array_keys($objRelated->getFields()) as $strField)
{
$arrFields[] = 'j' . $intCount . '.' . $strField . ' AS ' . $strKey . '__' . $strField;
$arrFields[] = 'j' . $intCount . '.' . \Database::quoteIdentifier($strField) . ' AS ' . $strKey . '__' . $strField;
}

$arrJoins[] = " LEFT JOIN " . $arrConfig['table'] . " j$intCount ON " . $arrOptions['table'] . "." . $strKey . "=j$intCount." . $arrConfig['field'];
$arrJoins[] = " LEFT JOIN " . $arrConfig['table'] . " j$intCount ON " . $arrOptions['table'] . "." . \Database::quoteIdentifier($strKey) . "=j$intCount." . $arrConfig['field'];
}
}
}
Expand All @@ -68,7 +68,7 @@ public static function find(array $arrOptions)
// Where condition
if ($arrOptions['column'] !== null)
{
$strQuery .= " WHERE " . (is_array($arrOptions['column']) ? implode(" AND ", $arrOptions['column']) : $arrOptions['table'] . '.' . $arrOptions['column'] . "=?");
$strQuery .= " WHERE " . (is_array($arrOptions['column']) ? implode(" AND ", $arrOptions['column']) : $arrOptions['table'] . '.' . \Database::quoteIdentifier($arrOptions['column']) . "=?");
}

// Group by
Expand Down Expand Up @@ -106,7 +106,7 @@ public static function count(array $arrOptions)

if ($arrOptions['column'] !== null)
{
$strQuery .= " WHERE " . (is_array($arrOptions['column']) ? implode(" AND ", $arrOptions['column']) : $arrOptions['table'] . '.' . $arrOptions['column'] . "=?");
$strQuery .= " WHERE " . (is_array($arrOptions['column']) ? implode(" AND ", $arrOptions['column']) : $arrOptions['table'] . '.' . \Database::quoteIdentifier($arrOptions['column']) . "=?");
}

return $strQuery;
Expand Down
2 changes: 1 addition & 1 deletion system/modules/core/library/Contao/User.php
Expand Up @@ -522,7 +522,7 @@ protected function checkAccountStatus()
*/
public function findBy($strColumn, $varValue)
{
$objResult = $this->Database->prepare("SELECT * FROM " . $this->strTable . " WHERE " . $strColumn . "=?")
$objResult = $this->Database->prepare("SELECT * FROM " . $this->strTable . " WHERE " . \Database::quoteIdentifier($strColumn) . "=?")
->limit(1)
->execute($varValue);

Expand Down
2 changes: 1 addition & 1 deletion system/modules/core/widgets/FileSelector.php
Expand Up @@ -181,7 +181,7 @@ public function generateAjax($folder, $strField, $level, $mount=false)
break;
}

$objField = $this->Database->prepare("SELECT " . $this->strField . " FROM " . $this->strTable . " WHERE id=?")
$objField = $this->Database->prepare("SELECT " . \Database::quoteIdentifier($this->strField) . " FROM " . $this->strTable . " WHERE id=?")
->limit(1)
->execute($this->strId);

Expand Down
2 changes: 1 addition & 1 deletion system/modules/core/widgets/FileTree.php
Expand Up @@ -66,7 +66,7 @@ public function __construct($arrAttributes=null)
$this->strOrderName = $this->orderField . str_replace($this->strField, '', $this->strName);

// Retrieve the order value
$objRow = $this->Database->prepare("SELECT {$this->orderField} FROM {$this->strTable} WHERE id=?")
$objRow = $this->Database->prepare("SELECT ".\Database::quoteIdentifier($this->orderField)." FROM {$this->strTable} WHERE id=?")
->limit(1)
->execute($this->activeRecord->id);

Expand Down
2 changes: 1 addition & 1 deletion system/modules/core/widgets/PageSelector.php
Expand Up @@ -271,7 +271,7 @@ public function generateAjax($id, $strField, $level)
break;
}

$objField = $this->Database->prepare("SELECT " . $this->strField . " FROM " . $this->strTable . " WHERE id=?")
$objField = $this->Database->prepare("SELECT " . \Database::quoteIdentifier($this->strField) . " FROM " . $this->strTable . " WHERE id=?")
->limit(1)
->execute($this->strId);

Expand Down
2 changes: 1 addition & 1 deletion system/modules/core/widgets/PageTree.php
Expand Up @@ -64,7 +64,7 @@ public function __construct($arrAttributes=null)
$this->strOrderName = $this->orderField . str_replace($this->strField, '', $this->strName);

// Retrieve the order value
$objRow = $this->Database->prepare("SELECT {$this->orderField} FROM {$this->strTable} WHERE id=?")
$objRow = $this->Database->prepare("SELECT ".\Database::quoteIdentifier($this->orderField)." FROM {$this->strTable} WHERE id=?")
->limit(1)
->execute($this->activeRecord->id);

Expand Down
10 changes: 5 additions & 5 deletions system/modules/listing/modules/ModuleListing.php
Expand Up @@ -120,7 +120,7 @@ protected function compile()
if ($strSearch && $strFor)
{
$varKeyword = '%' . $strFor . '%';
$strWhere = (!$this->list_where ? " WHERE " : " AND ") . $strSearch . " LIKE ?";
$strWhere = (!$this->list_where ? " WHERE " : " AND ") . \Database::quoteIdentifier($strSearch) . " LIKE ?";
}

foreach ($arrSearchFields as $field)
Expand Down Expand Up @@ -159,11 +159,11 @@ protected function compile()
}

// Get the selected records
$strQuery = "SELECT " . $this->strPk . "," . $this->list_fields;
$strQuery = "SELECT " . \Database::quoteIdentifier($this->strPk) . "," . implode(',', array_map('Database::quoteIdentifier', trimsplit(',', $this->list_fields)));

if ($this->list_info_where)
{
$strQuery .= ", (SELECT COUNT(*) FROM " . $this->list_table . " t2 WHERE t2." . $this->strPk . "=t1." . $this->strPk . " AND " . $this->list_info_where . ") AS _details";
$strQuery .= ", (SELECT COUNT(*) FROM " . $this->list_table . " t2 WHERE t2." . \Database::quoteIdentifier($this->strPk) . "=t1." . \Database::quoteIdentifier($this->strPk) . " AND " . $this->list_info_where . ") AS _details";
}

$strQuery .= " FROM " . $this->list_table . " t1";
Expand Down Expand Up @@ -203,7 +203,7 @@ protected function compile()
}
else
{
$strQuery .= " ORDER BY " . $order_by . ' ' . $sort;
$strQuery .= " ORDER BY " . \Database::quoteIdentifier($order_by) . ' ' . $sort;
}
}
elseif ($this->list_sort)
Expand Down Expand Up @@ -372,7 +372,7 @@ protected function listSingleRecord($id)
$this->list_info = deserialize($this->list_info);
$this->list_info_where = $this->replaceInsertTags($this->list_info_where, false);

$objRecord = $this->Database->prepare("SELECT " . $this->list_info . " FROM " . $this->list_table . " WHERE " . (($this->list_info_where != '') ? "(" . $this->list_info_where . ") AND " : "") . $this->strPk . "=?")
$objRecord = $this->Database->prepare("SELECT " . implode(',', array_map('Database::quoteIdentifier', trimsplit(',', $this->list_info))) . " FROM " . $this->list_table . " WHERE " . (($this->list_info_where != '') ? "(" . $this->list_info_where . ") AND " : "") . \Database::quoteIdentifier($this->strPk) . "=?")
->limit(1)
->execute($id);

Expand Down