Fix _validatePost returns true when empty form is submitted

Backport of #10625
This commit is contained in:
chinpei215 2017-05-06 21:43:51 +09:00
parent 68432f77de
commit a97bd234ee
2 changed files with 29 additions and 4 deletions

View file

@ -227,7 +227,7 @@ class SecurityComponent extends Component {
public function startup(Controller $controller) { public function startup(Controller $controller) {
$this->request = $controller->request; $this->request = $controller->request;
$this->_action = $controller->request->params['action']; $this->_action = $controller->request->params['action'];
$hasData = !empty($controller->request->data); $hasData = ($controller->request->data || $controller->request->is(array('put', 'post', 'delete', 'patch')));
try { try {
$this->_methodsRequired($controller); $this->_methodsRequired($controller);
$this->_secureRequired($controller); $this->_secureRequired($controller);
@ -487,9 +487,6 @@ class SecurityComponent extends Component {
* @return bool true if submitted form is valid * @return bool true if submitted form is valid
*/ */
protected function _validatePost(Controller $controller) { protected function _validatePost(Controller $controller) {
if (empty($controller->request->data)) {
return true;
}
$token = $this->_validToken($controller); $token = $this->_validToken($controller);
$hashParts = $this->_hashParts($controller); $hashParts = $this->_hashParts($controller);
$check = Security::hash(implode('', $hashParts), 'sha1'); $check = Security::hash(implode('', $hashParts), 'sha1');

View file

@ -279,6 +279,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'GET'; $_SERVER['REQUEST_METHOD'] = 'GET';
$this->Controller->request['action'] = 'posted'; $this->Controller->request['action'] = 'posted';
$this->Controller->Security->requirePost(array('posted')); $this->Controller->Security->requirePost(array('posted'));
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertTrue($this->Controller->failed); $this->assertTrue($this->Controller->failed);
} }
@ -292,6 +293,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REQUEST_METHOD'] = 'POST';
$this->Controller->request['action'] = 'posted'; $this->Controller->request['action'] = 'posted';
$this->Controller->Security->requirePost('posted'); $this->Controller->Security->requirePost('posted');
$this->Controller->Security->validatePost = false;
$this->Security->startup($this->Controller); $this->Security->startup($this->Controller);
$this->assertFalse($this->Controller->failed); $this->assertFalse($this->Controller->failed);
} }
@ -306,6 +308,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REQUEST_METHOD'] = 'POST';
$this->Controller->request['action'] = 'posted'; $this->Controller->request['action'] = 'posted';
$this->Controller->Security->requireSecure(array('posted')); $this->Controller->Security->requireSecure(array('posted'));
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertTrue($this->Controller->failed); $this->assertTrue($this->Controller->failed);
} }
@ -394,6 +397,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'GET'; $_SERVER['REQUEST_METHOD'] = 'GET';
$this->Controller->request['action'] = 'getted'; $this->Controller->request['action'] = 'getted';
$this->Controller->Security->requirePost('posted'); $this->Controller->Security->requirePost('posted');
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertFalse($this->Controller->failed); $this->assertFalse($this->Controller->failed);
} }
@ -407,6 +411,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REQUEST_METHOD'] = 'POST';
$this->Controller->request['action'] = 'getted'; $this->Controller->request['action'] = 'getted';
$this->Controller->Security->requireGet(array('getted')); $this->Controller->Security->requireGet(array('getted'));
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertTrue($this->Controller->failed); $this->assertTrue($this->Controller->failed);
} }
@ -420,6 +425,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'GET'; $_SERVER['REQUEST_METHOD'] = 'GET';
$this->Controller->request['action'] = 'getted'; $this->Controller->request['action'] = 'getted';
$this->Controller->Security->requireGet('getted'); $this->Controller->Security->requireGet('getted');
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertFalse($this->Controller->failed); $this->assertFalse($this->Controller->failed);
} }
@ -433,6 +439,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REQUEST_METHOD'] = 'POST';
$this->Controller->request['action'] = 'posted'; $this->Controller->request['action'] = 'posted';
$this->Security->requireGet('getted'); $this->Security->requireGet('getted');
$this->Security->validatePost = false;
$this->Security->startup($this->Controller); $this->Security->startup($this->Controller);
$this->assertFalse($this->Controller->failed); $this->assertFalse($this->Controller->failed);
} }
@ -446,6 +453,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REQUEST_METHOD'] = 'POST';
$this->Controller->request['action'] = 'putted'; $this->Controller->request['action'] = 'putted';
$this->Controller->Security->requirePut(array('putted')); $this->Controller->Security->requirePut(array('putted'));
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertTrue($this->Controller->failed); $this->assertTrue($this->Controller->failed);
} }
@ -459,6 +467,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'PUT'; $_SERVER['REQUEST_METHOD'] = 'PUT';
$this->Controller->request['action'] = 'putted'; $this->Controller->request['action'] = 'putted';
$this->Controller->Security->requirePut('putted'); $this->Controller->Security->requirePut('putted');
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertFalse($this->Controller->failed); $this->assertFalse($this->Controller->failed);
} }
@ -472,6 +481,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REQUEST_METHOD'] = 'POST';
$this->Controller->request['action'] = 'posted'; $this->Controller->request['action'] = 'posted';
$this->Controller->Security->requirePut('putted'); $this->Controller->Security->requirePut('putted');
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertFalse($this->Controller->failed); $this->assertFalse($this->Controller->failed);
} }
@ -485,6 +495,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REQUEST_METHOD'] = 'POST';
$this->Controller->request['action'] = 'deleted'; $this->Controller->request['action'] = 'deleted';
$this->Controller->Security->requireDelete(array('deleted', 'other_method')); $this->Controller->Security->requireDelete(array('deleted', 'other_method'));
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertTrue($this->Controller->failed); $this->assertTrue($this->Controller->failed);
} }
@ -498,6 +509,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'DELETE'; $_SERVER['REQUEST_METHOD'] = 'DELETE';
$this->Controller->request['action'] = 'deleted'; $this->Controller->request['action'] = 'deleted';
$this->Controller->Security->requireDelete('deleted'); $this->Controller->Security->requireDelete('deleted');
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertFalse($this->Controller->failed); $this->assertFalse($this->Controller->failed);
} }
@ -511,6 +523,7 @@ class SecurityComponentTest extends CakeTestCase {
$_SERVER['REQUEST_METHOD'] = 'POST'; $_SERVER['REQUEST_METHOD'] = 'POST';
$this->Controller->request['action'] = 'posted'; $this->Controller->request['action'] = 'posted';
$this->Controller->Security->requireDelete('deleted'); $this->Controller->Security->requireDelete('deleted');
$this->Controller->Security->validatePost = false;
$this->Controller->Security->startup($this->Controller); $this->Controller->Security->startup($this->Controller);
$this->assertFalse($this->Controller->failed); $this->assertFalse($this->Controller->failed);
} }
@ -606,6 +619,21 @@ class SecurityComponentTest extends CakeTestCase {
$this->assertFalse($result, 'validatePost passed when fields were missing. %s'); $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 * Test that objects can't be passed into the serialized string. This was a vector for RFI and LFI
* attacks. Thanks to Felix Wilhelm * attacks. Thanks to Felix Wilhelm