CRM-18493 Ensure that sort param is valid for use in mysql

diff --git a/api/v3/utils.php b/api/v3/utils.php
index 3754a00017f4961b16ef547a68fd6d87c469d49a..1cd0214a083656355552b96b803ba548f7f85147 100644 (file)
--- a/api/v3/utils.php
+++ b/api/v3/utils.php
@@ -669,6 +669,7 @@ function _civicrm_api3_get_query_object($params, $mode, $entity) {
   $sql = "$select $from $where $having";
 
   if (!empty($sort)) {
+    $sort = CRM_Utils_Type::escape($sort, 'MysqlOrderBy');
     $sql .= " ORDER BY $sort ";
   }
   if (!empty($rowCount)) {
@@ -881,11 +882,31 @@ function _civicrm_api3_get_options_from_params(&$params, $queryObject = FALSE, $
 
   $options = array(
     'offset' => CRM_Utils_Rule::integer($offset) ? $offset : NULL,
-    'sort' => CRM_Utils_Rule::string($sort) ? $sort : NULL,
     'limit' => CRM_Utils_Rule::integer($limit) ? $limit : NULL,
     'is_count' => $is_count,
     'return' => !empty($returnProperties) ? $returnProperties : array(),
   );
+  $finalSort = array();
+  if (is_array($sort)) {
+    foreach ($sort as $s) {
+      if (CRM_Utils_Rule::mysqlOrderBy($s)) {
+        $finalSort[] = $s;
+      }
+      else {
+        throw new API_Exception("Unknown field specified for sort. Cannot order by '$s'");
+      }
+    }
+  }
+  elseif ($sort) {
+    if (CRM_Utils_Rule::mysqlOrderBy($sort)) {
+      $finalSort[] = $sort;
+    }
+    else {
+      throw new API_Exception("Unknown field specified for sort. Cannot order by '$sort'");
+    }
+  }
+
+  $options['sort'] = !empty($finalSort) ? implode(', ', $finalSort) : NULL;
 
   if ($options['sort'] && stristr($options['sort'], 'SELECT')) {
     throw new API_Exception('invalid string in sort options');
@@ -940,6 +961,7 @@ function _civicrm_api3_apply_options_to_dao(&$params, &$dao, $entity) {
       $dao->limit((int) $options['offset'], (int) $options['limit']);
     }
     if (!empty($options['sort'])) {
+      $options['sort'] = CRM_Utils_Type::escape($options['sort'], 'MysqlOrderBy');
       $dao->orderBy($options['sort']);
     }
   }

diff --git a/tests/phpunit/api/v3/SyntaxConformanceTest.php b/tests/phpunit/api/v3/SyntaxConformanceTest.php
index a985c7c417659e29db73eb5ac754200b8bcf1186..e263aad599e9c970eadac49ad8374d3cf1f2139a 100644 (file)
--- a/tests/phpunit/api/v3/SyntaxConformanceTest.php
+++ b/tests/phpunit/api/v3/SyntaxConformanceTest.php
@@ -1037,6 +1037,21 @@ class api_v3_SyntaxConformanceTest extends CiviUnitTestCase {
     civicrm_api3($Entity, 'Create');
   }
 
+  /**
+   * @dataProvider entities_create
+   *
+   * Check that create doesn't work with an invalid
+   * @param $Entity
+   * @throws \PHPUnit_Framework_IncompleteTestError
+   */
+  public function testInvalidSort_get($Entity) {
+    $invalidEntitys = array('ActivityType', 'Setting', 'System');
+    if (in_array($Entity, $invalidEntitys)) {
+      $this->markTestSkipped('It seems OK for setting to skip here as it silently sips invalid params');
+    }
+    $result = $this->callAPIFailure($Entity, 'get', array('options' => array('sort' => 'sleep(1)')));
+  }
+
   /**
    * @dataProvider entities_create
    *