From f66dec8a96999a5ef042d9a72dec6784ac56fcc6 Mon Sep 17 00:00:00 2001
From: mark_story <mark@mark-story.com>
Date: Sun, 10 Dec 2017 21:44:47 -0500
Subject: [PATCH] Make postConditions() less permissive.

We were notified by `ooooooo_q` that postConditions() is vulnerable to
SQL injection if used without SecurityComponent tampering prevention.

This change attempts to make postConditions() safer by exploding in
unsafe scenarios.
---
 lib/Cake/Controller/Controller.php            | 10 +++++-
 .../Test/Case/Controller/ControllerTest.php   | 36 +++++++++++++++++++
 2 files changed, 45 insertions(+), 1 deletion(-)

diff --git a/lib/Cake/Controller/Controller.php b/lib/Cake/Controller/Controller.php
index 41c848a7d..7dd2a4ae1 100644
--- a/lib/Cake/Controller/Controller.php
+++ b/lib/Cake/Controller/Controller.php
@@ -1018,7 +1018,12 @@ class Controller extends CakeObject implements CakeEventListener {
 	}
 
 /**
- * Converts POST'ed form data to a model conditions array, suitable for use in a Model::find() call.
+ * Converts POST'ed form data to a model conditions array. 
+ *
+ * If combined with SecurityComponent these conditions could be suitable
+ * for use in a Model::find() call. Without SecurityComponent this method
+ * is vulnerable creating conditions containing SQL injection. While we
+ * attempt to raise exceptions.
  *
  * @param array $data POST'ed data organized by model and field
  * @param string|array $op A string containing an SQL comparison operator, or an array matching operators
@@ -1046,6 +1051,9 @@ class Controller extends CakeObject implements CakeEventListener {
 		$arrayOp = is_array($op);
 		foreach ($data as $model => $fields) {
 			foreach ($fields as $field => $value) {
+				if (preg_match('#[!=><~\&\|\)\(]#', $field)) {
+					throw new RuntimeException("Unsafe operator found in {$model}.{$field}");
+				}
 				$key = $model . '.' . $field;
 				$fieldOp = $op;
 				if ($arrayOp) {
diff --git a/lib/Cake/Test/Case/Controller/ControllerTest.php b/lib/Cake/Test/Case/Controller/ControllerTest.php
index ef157ddd1..26e3c6b16 100644
--- a/lib/Cake/Test/Case/Controller/ControllerTest.php
+++ b/lib/Cake/Test/Case/Controller/ControllerTest.php
@@ -1177,6 +1177,42 @@ class ControllerTest extends CakeTestCase {
 		$this->assertSame($expected, $result);
 	}
 
+/**
+ * data provider for dangerous post conditions.
+ *
+ * @return array
+ */
+    public function dangerousPostConditionsProvider() {
+		return array(
+			array(
+				array('Model' => array('field !=' => 1))
+			),
+			array(
+				array('Model' => array('field AND 1=1 OR' => 'thing'))
+			),
+			array(
+				array('Model' => array('field >' => 1))
+			),
+			array(
+				array('Model' => array('field OR RAND()' => 1))
+			),
+		);
+	}
+
+/**
+ * test postConditions raising an exception on unsafe keys.
+ *
+ * @expectedException RuntimeException
+ * @dataProvider dangerousPostConditionsProvider
+ * @return void
+ */
+	public function testPostConditionsDangerous($data) {
+		$request = new CakeRequest('controller_posts/index');
+
+		$Controller = new Controller($request);
+		$Controller->postConditions($data);
+	}
+
 /**
  * testControllerHttpCodes method
  *