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

3 files changed, 63 insertions, 16 deletions

diff --git a/includes/database/database.inc b/includes/database/database.inc
index d0348cf7c..24eed8af2 100644
--- a/includes/database/database.inc
+++ b/includes/database/database.inc
@@ -779,6 +779,20 @@ abstract class DatabaseConnection extends PDO {
   }
 
   /**
+   * 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.
    *
    * The wildcard characters "%" and "_" as well as backslash are prefixed with
@@ -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.
  *
@@ -2438,6 +2452,21 @@ function db_escape_table($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.
  *
  * The wildcard characters "%" and "_" as well as backslash are prefixed with
diff --git a/includes/database/query.inc b/includes/database/query.inc
index 5fa846548..43c54f8a8 100644
--- a/includes/database/query.inc
+++ b/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 @@ class DatabaseCondition implements QueryConditionInterface, Countable {
                 $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'] . ') ';
           }
         }
       }
diff --git a/includes/database/select.inc b/includes/database/select.inc
index 2961bb1ef..48bd04daf 100644
--- a/includes/database/select.inc
+++ b/includes/database/select.inc
@@ -445,6 +445,29 @@ interface SelectQueryInterface extends QueryConditionInterface, QueryAlterableIn
   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.
    *
    * Select queries have dependent objects that must be deep-cloned.  The
@@ -906,10 +929,7 @@ class SelectQuery extends Query implements SelectQueryInterface {
   /* 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 @@ class SelectQuery extends Query implements SelectQueryInterface {
 
   /* 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 @@ class SelectQuery extends Query implements SelectQueryInterface {
     $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 @@ class SelectQuery extends Query implements SelectQueryInterface {
 
       // 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'];