From a97bd234ee13c0a1fcd0cea4e5c693818578cd1b Mon Sep 17 00:00:00 2001 From: chinpei215 Date: Sat, 6 May 2017 21:43:51 +0900 Subject: [PATCH] Fix _validatePost returns true when empty form is submitted Backport of #10625 --- .../Component/SecurityComponent.php | 5 +--- .../Component/SecurityComponentTest.php | 28 +++++++++++++++++++ 2 files changed, 29 insertions(+), 4 deletions(-) diff --git a/lib/Cake/Controller/Component/SecurityComponent.php b/lib/Cake/Controller/Component/SecurityComponent.php index 5606488ec98..f7aca283eb0 100644 --- a/lib/Cake/Controller/Component/SecurityComponent.php +++ b/lib/Cake/Controller/Component/SecurityComponent.php @@ -227,7 +227,7 @@ class SecurityComponent extends Component { public function startup(Controller $controller) { $this->request = $controller->request; $this->_action = $controller->request->params['action']; - $hasData = !empty($controller->request->data); + $hasData = ($controller->request->data || $controller->request->is(array('put', 'post', 'delete', 'patch'))); try { $this->_methodsRequired($controller); $this->_secureRequired($controller); @@ -487,9 +487,6 @@ protected function _authRequired(Controller $controller) { * @return bool true if submitted form is valid */ protected function _validatePost(Controller $controller) { - if (empty($controller->request->data)) { - return true; - } $token = $this->_validToken($controller); $hashParts = $this->_hashParts($controller); $check = Security::hash(implode('', $hashParts), 'sha1'); diff --git a/lib/Cake/Test/Case/Controller/Component/SecurityComponentTest.php b/lib/Cake/Test/Case/Controller/Component/SecurityComponentTest.php index 1041a8195b2..f232d2139ea 100644 --- a/lib/Cake/Test/Case/Controller/Component/SecurityComponentTest.php +++ b/lib/Cake/Test/Case/Controller/Component/SecurityComponentTest.php @@ -279,6 +279,7 @@ public function testRequirePostFail() { $_SERVER['REQUEST_METHOD'] = 'GET'; $this->Controller->request['action'] = 'posted'; $this->Controller->Security->requirePost(array('posted')); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertTrue($this->Controller->failed); } @@ -292,6 +293,7 @@ public function testRequirePostSucceed() { $_SERVER['REQUEST_METHOD'] = 'POST'; $this->Controller->request['action'] = 'posted'; $this->Controller->Security->requirePost('posted'); + $this->Controller->Security->validatePost = false; $this->Security->startup($this->Controller); $this->assertFalse($this->Controller->failed); } @@ -306,6 +308,7 @@ public function testRequireSecureFail() { $_SERVER['REQUEST_METHOD'] = 'POST'; $this->Controller->request['action'] = 'posted'; $this->Controller->Security->requireSecure(array('posted')); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertTrue($this->Controller->failed); } @@ -394,6 +397,7 @@ public function testRequirePostSucceedWrongMethod() { $_SERVER['REQUEST_METHOD'] = 'GET'; $this->Controller->request['action'] = 'getted'; $this->Controller->Security->requirePost('posted'); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertFalse($this->Controller->failed); } @@ -407,6 +411,7 @@ public function testRequireGetFail() { $_SERVER['REQUEST_METHOD'] = 'POST'; $this->Controller->request['action'] = 'getted'; $this->Controller->Security->requireGet(array('getted')); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertTrue($this->Controller->failed); } @@ -420,6 +425,7 @@ public function testRequireGetSucceed() { $_SERVER['REQUEST_METHOD'] = 'GET'; $this->Controller->request['action'] = 'getted'; $this->Controller->Security->requireGet('getted'); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertFalse($this->Controller->failed); } @@ -433,6 +439,7 @@ public function testRequireGetSucceedWrongMethod() { $_SERVER['REQUEST_METHOD'] = 'POST'; $this->Controller->request['action'] = 'posted'; $this->Security->requireGet('getted'); + $this->Security->validatePost = false; $this->Security->startup($this->Controller); $this->assertFalse($this->Controller->failed); } @@ -446,6 +453,7 @@ public function testRequirePutFail() { $_SERVER['REQUEST_METHOD'] = 'POST'; $this->Controller->request['action'] = 'putted'; $this->Controller->Security->requirePut(array('putted')); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertTrue($this->Controller->failed); } @@ -459,6 +467,7 @@ public function testRequirePutSucceed() { $_SERVER['REQUEST_METHOD'] = 'PUT'; $this->Controller->request['action'] = 'putted'; $this->Controller->Security->requirePut('putted'); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertFalse($this->Controller->failed); } @@ -472,6 +481,7 @@ public function testRequirePutSucceedWrongMethod() { $_SERVER['REQUEST_METHOD'] = 'POST'; $this->Controller->request['action'] = 'posted'; $this->Controller->Security->requirePut('putted'); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertFalse($this->Controller->failed); } @@ -485,6 +495,7 @@ public function testRequireDeleteFail() { $_SERVER['REQUEST_METHOD'] = 'POST'; $this->Controller->request['action'] = 'deleted'; $this->Controller->Security->requireDelete(array('deleted', 'other_method')); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertTrue($this->Controller->failed); } @@ -498,6 +509,7 @@ public function testRequireDeleteSucceed() { $_SERVER['REQUEST_METHOD'] = 'DELETE'; $this->Controller->request['action'] = 'deleted'; $this->Controller->Security->requireDelete('deleted'); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertFalse($this->Controller->failed); } @@ -511,6 +523,7 @@ public function testRequireDeleteSucceedWrongMethod() { $_SERVER['REQUEST_METHOD'] = 'POST'; $this->Controller->request['action'] = 'posted'; $this->Controller->Security->requireDelete('deleted'); + $this->Controller->Security->validatePost = false; $this->Controller->Security->startup($this->Controller); $this->assertFalse($this->Controller->failed); } @@ -606,6 +619,21 @@ public function testValidatePostFormHacking() { $this->assertFalse($result, 'validatePost passed when fields were missing. %s'); } +/** + * testValidatePostEmptyForm method + * + * Test that validatePost fails if empty form is submitted. + * + * @return void + */ + public function testValidatePostEmptyForm() { + $_SERVER['REQUEST_METHOD'] = 'POST'; + $this->Controller->request->data = array(); + $this->Controller->Security->startup($this->Controller); + $result = $this->validatePost('AuthSecurityException', '\'_Token\' was not found in request data.'); + $this->assertFalse($result, 'validatePost passed when empty form is submitted'); + } + /** * Test that objects can't be passed into the serialized string. This was a vector for RFI and LFI * attacks. Thanks to Felix Wilhelm