- Patch #769554 by jpmckinney, Berdir, cha0s: fixed trivial SQL injection(s) with SelectQuery.

includes/database/database.inc

@@ -778,6 +778,20 @@ public function escapeTable($table) {
     return preg_replace('/[^A-Za-z0-9_.]+/', '', $table);
   }
 
+  /**
+   * Escapes a field name string.
+   *
+   * Force all field names to be strictly alphanumeric-plus-underscore.
+   * For some database drivers, it may also wrap the field name in
+   * database-specific escape characters.
+   *
+   * @return
+   *   The sanitized field name string.
+   */
+  public function escapeField($field) {
+    return preg_replace('/[^A-Za-z0-9_.]+/', '', $field);
+  }
+
   /**
    * Escapes characters that work as wildcard characters in a LIKE pattern.
    *
@@ -2423,7 +2437,7 @@ function db_set_active($key = 'default') {
 }
 
 /**
- * Restricts a dynamic table, column, or constraint name to safe characters.
+ * Restricts a dynamic table name to safe characters.
  *
  * Only keeps alphanumeric and underscores.
  *
@@ -2437,6 +2451,21 @@ function db_escape_table($table) {
   return Database::getConnection()->escapeTable($table);
 }
 
+/**
+ * Restricts a dynamic column or constraint name to safe characters.
+ *
+ * Only keeps alphanumeric and underscores.
+ *
+ * @param $field
+ *   The field name to escape.
+ *
+ * @return
+ *   The escaped field name as a string.
+ */
+function db_escape_field($field) {
+  return Database::getConnection()->escapeField($field);
+}
+
 /**
  * Escapes characters that work as wildcard characters in a LIKE pattern.
  *

includes/database/query.inc

@@ -21,10 +21,11 @@ interface QueryConditionInterface {
    *
    * This method can take a variable number of parameters. If called with two
    * parameters, they are taken as $field and $value with $operator having a value
-   * of =.
+   * of IN if $value is an array and = otherwise.
    *
    * @param $field
-   *   The name of the field to check.
+   *   The name of the field to check. If you would like to add a more complex
+   *   condition involving operators or functions, use where().
    * @param $value
    *   The value to test the field against. In most cases, this is a scalar. For more
    *   complex options, it is an array. The meaning of each element in the array is
@@ -1255,7 +1256,7 @@ public function compile(DatabaseConnection $connection, QueryPlaceholderInterfac
                 $placeholders[] = $placeholder;
               }
             }
-            $condition_fragments[] = ' (' . $condition['field'] . ' ' . $operator['operator'] . ' ' . $operator['prefix'] . implode($operator['delimiter'], $placeholders) . $operator['postfix'] . ') ';
+            $condition_fragments[] = ' (' . $connection->escapeField($condition['field']) . ' ' . $operator['operator'] . ' ' . $operator['prefix'] . implode($operator['delimiter'], $placeholders) . $operator['postfix'] . ') ';
           }
         }
       }

includes/database/select.inc

@@ -444,6 +444,29 @@ public function isPrepared();
    */
   public function preExecute(SelectQueryInterface $query = NULL);
 
+  /**
+   * Helper function to build most common HAVING conditional clauses.
+   *
+   * This method can take a variable number of parameters. If called with two
+   * parameters, they are taken as $field and $value with $operator having a value
+   * of IN if $value is an array and = otherwise.
+   *
+   * @param $field
+   *   The name of the field to check. If you would like to add a more complex
+   *   condition involving operators or functions, use having().
+   * @param $value
+   *   The value to test the field against. In most cases, this is a scalar. For more
+   *   complex options, it is an array. The meaning of each element in the array is
+   *   dependent on the $operator.
+   * @param $operator
+   *   The comparison operator, such as =, <, or >=. It also accepts more complex
+   *   options such as IN, LIKE, or BETWEEN. Defaults to IN if $value is an array
+   *   = otherwise.
+   * @return QueryConditionInterface
+   *   The called object.
+   */
+  public function havingCondition($field, $value = NULL, $operator = NULL);
+
   /**
    * Clone magic method.
    *
@@ -906,10 +929,7 @@ public function getMetaData($key) {
   /* Implementations of QueryConditionInterface for the WHERE clause. */
 
   public function condition($field, $value = NULL, $operator = NULL) {
-    if (!isset($num_args)) {
-      $num_args = func_num_args();
-    }
-    $this->where->condition($field, $value, $operator, $num_args);
+    $this->where->condition($field, $value, $operator);
     return $this;
   }
 
@@ -942,11 +962,8 @@ public function compile(DatabaseConnection $connection, QueryPlaceholderInterfac
 
   /* Implementations of QueryConditionInterface for the HAVING clause. */
 
-  public function havingCondition($field, $value = NULL, $operator = '=') {
-    if (!isset($num_args)) {
-      $num_args = func_num_args();
-    }
-    $this->having->condition($field, $value, $operator, $num_args);
+  public function havingCondition($field, $value = NULL, $operator = NULL) {
+    $this->having->condition($field, $value, $operator);
     return $this;
   }
 
@@ -1296,13 +1313,13 @@ public function __toString() {
     $fields = array();
     foreach ($this->tables as $alias => $table) {
       if (!empty($table['all_fields'])) {
-        $fields[] = $alias . '.*';
+        $fields[] = $this->connection->escapeTable($alias) . '.*';
       }
     }
     foreach ($this->fields as $alias => $field) {
       // Always use the AS keyword for field aliases, as some
       // databases require it (e.g., PostgreSQL).
-      $fields[] = (isset($field['table']) ? $field['table'] . '.' : '') . $field['field'] . ' AS ' . $field['alias'];
+      $fields[] = (isset($field['table']) ? $this->connection->escapeTable($field['table']) . '.' : '') . $this->connection->escapeField($field['field']) . ' AS ' . $this->connection->escapeField($field['alias']);
     }
     foreach ($this->expressions as $alias => $expression) {
       $fields[] = $expression['expression'] . ' AS ' . $expression['alias'];
@@ -1331,7 +1348,7 @@ public function __toString() {
 
       // Don't use the AS keyword for table aliases, as some
       // databases don't support it (e.g., Oracle).
-      $query .=  $table_string . ' ' . $table['alias'];
+      $query .=  $table_string . ' ' . $this->connection->escapeTable($table['alias']);
 
       if (!empty($table['condition'])) {
         $query .= ' ON ' . $table['condition'];

modules/openid/openid.module

@@ -550,7 +550,7 @@ function openid_association($op_endpoint) {
 
   // Remove Old Associations:
   db_delete('openid_association')
-    ->condition('created + expires_in', REQUEST_TIME, '<')
+    ->where('created + expires_in < :request_time', array(':request_time' => REQUEST_TIME))
     ->execute();
 
   // Check to see if we have an association for this IdP already

modules/simpletest/tests/database_test.test

@@ -1835,7 +1835,7 @@ class DatabaseSelectComplexTestCase extends DatabaseTestCase {
     $task_field = $query->addField('t', 'task');
     $query->orderBy($count_field);
     $query->groupBy($task_field);
-    $query->havingCondition('COUNT(task)', 2, '>=');
+    $query->having('COUNT(task) >= 2');
     $result = $query->execute();
 
     $num_records = 0;