diff options
| -rw-r--r-- | CHANGELOG.txt | 3 | ||||
| -rw-r--r-- | includes/database/select.inc | 5 | ||||
| -rw-r--r-- | includes/tablesort.inc | 7 | ||||
| -rw-r--r-- | modules/simpletest/tests/database_test.test | 9 |
4 files changed, 19 insertions, 5 deletions
diff --git a/CHANGELOG.txt b/CHANGELOG.txt index 46096d302..97a04b3f6 100644 --- a/CHANGELOG.txt +++ b/CHANGELOG.txt @@ -1,6 +1,9 @@ Drupal 7.33, xxxx-xx-xx (development version) ----------------------- +- Security improvement: Made the database API's orderBy() method sanitize the + sort direction ("ASC" or "DESC") for queries built with db_select(), so that + calling code does not have to. - Changed the RDF module to consistently output RDF metadata for nodes and comments near where the node is rendered in the HTML (minor markup and data structure change). diff --git a/includes/database/select.inc b/includes/database/select.inc index 70c03a283..3abd205c9 100644 --- a/includes/database/select.inc +++ b/includes/database/select.inc @@ -377,7 +377,8 @@ interface SelectQueryInterface extends QueryConditionInterface, QueryAlterableIn * @param $field * The field on which to order. * @param $direction - * The direction to sort. Legal values are "ASC" and "DESC". + * The direction to sort. Legal values are "ASC" and "DESC". Any other value + * will be converted to "ASC". * @return SelectQueryInterface * The called object. */ @@ -1384,6 +1385,8 @@ class SelectQuery extends Query implements SelectQueryInterface { } public function orderBy($field, $direction = 'ASC') { + // Only allow ASC and DESC, default to ASC. + $direction = strtoupper($direction) == 'DESC' ? 'DESC' : 'ASC'; $this->order[$field] = $direction; return $this; } diff --git a/includes/tablesort.inc b/includes/tablesort.inc index e589526c6..e9c280686 100644 --- a/includes/tablesort.inc +++ b/includes/tablesort.inc @@ -46,10 +46,9 @@ class TableSort extends SelectQueryExtender { // Based on code from db_escape_table(), but this can also contain a dot. $field = preg_replace('/[^A-Za-z0-9_.]+/', '', $ts['sql']); - // Sort order can only be ASC or DESC. - $sort = drupal_strtoupper($ts['sort']); - $sort = in_array($sort, array('ASC', 'DESC')) ? $sort : ''; - $this->orderBy($field, $sort); + // orderBy() will ensure that only ASC/DESC values are accepted, so we + // don't need to sanitize that here. + $this->orderBy($field, $ts['sort']); } return $this; } diff --git a/modules/simpletest/tests/database_test.test b/modules/simpletest/tests/database_test.test index 209bf6813..a65cc64ec 100644 --- a/modules/simpletest/tests/database_test.test +++ b/modules/simpletest/tests/database_test.test @@ -1947,6 +1947,15 @@ class DatabaseSelectOrderedTestCase extends DatabaseTestCase { $this->assertEqual($num_records, 4, 'Returned the correct number of rows.'); } + + /** + * Tests that the sort direction is sanitized properly. + */ + function testOrderByEscaping() { + $query = db_select('test')->orderBy('name', 'invalid direction'); + $order_bys = $query->getOrderBy(); + $this->assertEqual($order_bys['name'], 'ASC', 'Invalid order by direction is converted to ASC.'); + } } /** |