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.
This commit is contained in:
mark_story 2016-01-20 21:17:27 -05:00
parent 983a2f65e8
commit 4b8d628a2e
2 changed files with 32 additions and 24 deletions

View file

@ -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']);
}
}

View file

@ -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(