Use a permitted list instead of a ban list.

This should be safer as we are more confident on what is coming in.
cakephp · Dec 13, 2017 · a9618f6 · a9618f6
1 parent f66dec8
commit a9618f6
Show file tree

Hide file tree

Showing 2 changed files with 6 additions and 2 deletions.
diff --git a/lib/Cake/Controller/Controller.php b/lib/Cake/Controller/Controller.php
@@ -1033,6 +1033,7 @@ public function flash($message, $url, $pause = 1, $layout = 'flash') {
  *        included in the returned conditions
  * @return array|null An array of model conditions
  * @deprecated 3.0.0 Will be removed in 3.0.
+ * @throws RuntimeException when unsafe operators are found.
  */
 	public function postConditions($data = array(), $op = null, $bool = 'AND', $exclusive = false) {
 		if (!is_array($data) || empty($data)) {
@@ -1051,7 +1052,7 @@ public function postConditions($data = array(), $op = null, $bool = 'AND', $excl
 		$arrayOp = is_array($op);
 		foreach ($data as $model => $fields) {
 			foreach ($fields as $field => $value) {
-				if (preg_match('#[!=><~\&\|\)\(]#', $field)) {
+				if (preg_match('#[^a-zA-Z0-9_ ]#', $field)) {
 					throw new RuntimeException("Unsafe operator found in {$model}.{$field}");
 				}
 				$key = $model . '.' . $field;

diff --git a/lib/Cake/Test/Case/Controller/ControllerTest.php b/lib/Cake/Test/Case/Controller/ControllerTest.php
@@ -1182,7 +1182,7 @@ public function testPostConditions() {
  *
  * @return array
  */
-    public function dangerousPostConditionsProvider() {
+	public function dangerousPostConditionsProvider() {
 		return array(
 			array(
 				array('Model' => array('field !=' => 1))
@@ -1196,6 +1196,9 @@ public function dangerousPostConditionsProvider() {
 			array(
 				array('Model' => array('field OR RAND()' => 1))
 			),
+			array(
+				array('Posts' => array('id IS NULL union all select posts.* from posts where id; --' => 1))
+			),
 		);
 	}