Skip to content

Commit

Permalink
Fix _validatePost returns true when empty form is submitted
Browse files Browse the repository at this point in the history 
Backport of #10625
  • Loading branch information
@chinpei215
chinpei215 committed May 6, 2017
1 parent 68432f7 commit a97bd23
Show file tree
Hide file tree
Showing 2 changed files with 29 additions and 4 deletions.
5 changes: 1 addition & 4 deletions lib/Cake/Controller/Component/SecurityComponent.php
View file
Expand Up @@ -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);
Expand Down Expand Up @@ -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');
Expand Down
28 changes: 28 additions & 0 deletions lib/Cake/Test/Case/Controller/Component/SecurityComponentTest.php
View file
Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand Down Expand Up @@ -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
Expand Down

0 comments on commit a97bd23

Please sign in to comment.