From 4b8d628a2e3a693d287e0b789b2ee42bc0829f4f Mon Sep 17 00:00:00 2001
From: mark_story <mark@mark-story.com>
Date: Wed, 20 Jan 2016 21:17:27 -0500
Subject: [PATCH] Backport SecurityComponent fixes from #8071 to 2.x

If the request manages to have data set outside of post/put we should
still validate the request body. This expands SecurityComponent to cover
PATCH and DELETE methods, as well as request methods that should be
safe, but somehow end up not safe.
---
 .../Component/SecurityComponent.php           |  6 +--
 .../Component/SecurityComponentTest.php       | 50 +++++++++++--------
 2 files changed, 32 insertions(+), 24 deletions(-)

diff --git a/lib/Cake/Controller/Component/SecurityComponent.php b/lib/Cake/Controller/Component/SecurityComponent.php
index 12f225ff3..31a14ef5c 100644
--- a/lib/Cake/Controller/Component/SecurityComponent.php
+++ b/lib/Cake/Controller/Component/SecurityComponent.php
@@ -224,7 +224,7 @@ class SecurityComponent extends Component {
 		$this->_secureRequired($controller);
 		$this->_authRequired($controller);
 
-		$isPost = $this->request->is(array('post', 'put'));
+		$hasData = !empty($this->request->data);
 		$isNotRequestAction = (
 			!isset($controller->request->params['requested']) ||
 			$controller->request->params['requested'] != 1
@@ -234,7 +234,7 @@ class SecurityComponent extends Component {
 			return $this->blackHole($controller, 'auth');
 		}
 
-		if (!in_array($this->_action, (array)$this->unlockedActions) && $isPost && $isNotRequestAction) {
+		if (!in_array($this->_action, (array)$this->unlockedActions) && $hasData && $isNotRequestAction) {
 			if ($this->validatePost && $this->_validatePost($controller) === false) {
 				return $this->blackHole($controller, 'auth');
 			}
@@ -243,7 +243,7 @@ class SecurityComponent extends Component {
 			}
 		}
 		$this->generateToken($controller->request);
-		if ($isPost && is_array($controller->request->data)) {
+		if ($hasData && is_array($controller->request->data)) {
 			unset($controller->request->data['_Token']);
 		}
 	}
diff --git a/lib/Cake/Test/Case/Controller/Component/SecurityComponentTest.php b/lib/Cake/Test/Case/Controller/Component/SecurityComponentTest.php
index 8220d8ead..112e5ec84 100644
--- a/lib/Cake/Test/Case/Controller/Component/SecurityComponentTest.php
+++ b/lib/Cake/Test/Case/Controller/Component/SecurityComponentTest.php
@@ -330,19 +330,23 @@ class SecurityComponentTest extends CakeTestCase {
  */
 	public function testRequireAuthSucceed() {
 		$_SERVER['REQUEST_METHOD'] = 'AUTH';
+		$this->Controller->Security->unlockedActions = array('posted');
 		$this->Controller->request['action'] = 'posted';
 		$this->Controller->Security->requireAuth('posted');
 		$this->Controller->Security->startup($this->Controller);
 		$this->assertFalse($this->Controller->failed);
 
 		$this->Controller->Security->Session->write('_Token', array(
-			'allowedControllers' => array('SecurityTest'), 'allowedActions' => array('posted')
+			'allowedControllers' => array('SecurityTest'),
+			'allowedActions' => array('posted')
 		));
 		$this->Controller->request['controller'] = 'SecurityTest';
 		$this->Controller->request['action'] = 'posted';
 
 		$this->Controller->request->data = array(
-			'username' => 'willy', 'password' => 'somePass', '_Token' => ''
+			'username' => 'willy',
+			'password' => 'somePass',
+			'_Token' => ''
 		);
 		$this->Controller->action = 'posted';
 		$this->Controller->Security->requireAuth('posted');
@@ -480,6 +484,29 @@ class SecurityComponentTest extends CakeTestCase {
 		$this->assertFalse($this->Controller->failed);
 	}
 
+/**
+ * Test that validatePost fires on GET with request data.
+ * This could happen when method overriding is used.
+ *
+ * @return void
+ * @triggers Controller.startup $this->Controller
+ */
+	public function testValidatePostOnGetWithData() {
+		$_SERVER['REQUEST_METHOD'] = 'GET';
+		$this->Controller->Security->startup($this->Controller);
+
+		$fields = 'an-invalid-token';
+		$unlocked = '';
+
+		$this->Controller->request->data = [
+			'Model' => array('username' => 'nate', 'password' => 'foo', 'valid' => '0'),
+			'_Token' => compact('fields', 'unlocked')
+		];
+		$this->assertFalse($this->Controller->failed, 'Should not be failed yet');
+		$this->Controller->Security->startup($this->Controller);
+		$this->assertTrue($this->Controller->failed, 'Should fail because of validatePost.');
+	}
+
 /**
  * Simple hash validation test
  *
@@ -1230,11 +1257,6 @@ class SecurityComponentTest extends CakeTestCase {
 
 		$this->Security->Session->write('_Token.csrfTokens', array('nonce1' => strtotime('+10 minutes')));
 
-		$this->Controller->request = $this->getMock('CakeRequest', array('is'));
-		$this->Controller->request->expects($this->once())->method('is')
-			->with(array('post', 'put'))
-			->will($this->returnValue(true));
-
 		$this->Controller->request->params['action'] = 'index';
 		$this->Controller->request->data = array(
 			'_Token' => array(
@@ -1299,11 +1321,6 @@ class SecurityComponentTest extends CakeTestCase {
 
 		$this->Security->Session->write('_Token.csrfTokens', array('nonce1' => strtotime('+10 minutes')));
 
-		$this->Controller->request = $this->getMock('CakeRequest', array('is'));
-		$this->Controller->request->expects($this->once())->method('is')
-			->with(array('post', 'put'))
-			->will($this->returnValue(true));
-
 		$this->Controller->request->params['action'] = 'index';
 		$this->Controller->request->data = array(
 			'_Token' => array(
@@ -1329,11 +1346,6 @@ class SecurityComponentTest extends CakeTestCase {
 
 		$this->Security->Session->write('_Token.csrfTokens', array('nonce1' => strtotime('-5 minutes')));
 
-		$this->Controller->request = $this->getMock('CakeRequest', array('is'));
-		$this->Controller->request->expects($this->once())->method('is')
-			->with(array('post', 'put'))
-			->will($this->returnValue(true));
-
 		$this->Controller->request->params['action'] = 'index';
 		$this->Controller->request->data = array(
 			'_Token' => array(
@@ -1386,10 +1398,6 @@ class SecurityComponentTest extends CakeTestCase {
 		$this->Security->Session->write('_Token.csrfTokens', array('nonce1' => strtotime('+10 minutes')));
 
 		$this->Controller->request = $this->getMock('CakeRequest', array('is'));
-		$this->Controller->request->expects($this->once())->method('is')
-			->with(array('post', 'put'))
-			->will($this->returnValue(true));
-
 		$this->Controller->request->params['action'] = 'index';
 		$this->Controller->request->data = array(
 			'_Token' => array(