Skip to content

Commit

Permalink
Only allow GET, HEAD, OPTIONS to not have CSRF tokens.
Browse files Browse the repository at this point in the history
This covers cases where bad guys make up fake HTTP methods to trick CSRF
validation.

Update test cases to not muck about in $_SERVER too.
  • Loading branch information
markstory committed Nov 26, 2015
1 parent f7f5e21 commit 0f818a2
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 27 deletions.
2 changes: 1 addition & 1 deletion src/Controller/Component/CsrfComponent.php
Expand Up @@ -94,7 +94,7 @@ public function startup(Event $event)
if ($request->is('get') && $cookieData === null) {
$this->_setCookie($request, $response);
}
if ($request->is(['patch', 'put', 'post', 'delete'])) {
if (!$request->is(['head', 'get', 'options'])) {
$this->_validateToken($request);
unset($request->data[$this->_config['field']]);
}
Expand Down
60 changes: 34 additions & 26 deletions tests/TestCase/Controller/Component/CsrfComponentTest.php
Expand Up @@ -61,10 +61,11 @@ public function tearDown()
*/
public function testSettingCookie()
{
$_SERVER['REQUEST_METHOD'] = 'GET';

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request(['webroot' => '/dir/']);
$controller->request = new Request([
'environment' => ['REQUEST_METHOD' => 'GET'],
'webroot' => '/dir/',
]);
$controller->response = new Response();

$event = new Event('Controller.startup', $controller);
Expand All @@ -87,7 +88,7 @@ public function testSettingCookie()
public static function httpMethodProvider()
{
return [
['PATCH'], ['PUT'], ['POST'], ['DELETE']
['PATCH'], ['PUT'], ['POST'], ['DELETE'], ['PURGE'], ['INVALIDMETHOD']
];
}

Expand All @@ -100,11 +101,14 @@ public static function httpMethodProvider()
*/
public function testValidTokenInHeader($method)
{
$_SERVER['REQUEST_METHOD'] = $method;
$_SERVER['HTTP_X_CSRF_TOKEN'] = 'testing123';

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request(['cookies' => ['csrfToken' => 'testing123']]);
$controller->request = new Request([
'environment' => [
'REQUEST_METHOD' => $method,
'HTTP_X_CSRF_TOKEN' => 'testing123',
],
'cookies' => ['csrfToken' => 'testing123']
]);
$controller->response = new Response();

$event = new Event('Controller.startup', $controller);
Expand All @@ -122,11 +126,12 @@ public function testValidTokenInHeader($method)
*/
public function testInvalidTokenInHeader($method)
{
$_SERVER['REQUEST_METHOD'] = $method;
$_SERVER['HTTP_X_CSRF_TOKEN'] = 'nope';

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request([
'environment' => [
'REQUEST_METHOD' => $method,
'HTTP_X_CSRF_TOKEN' => 'nope',
],
'cookies' => ['csrfToken' => 'testing123']
]);
$controller->response = new Response();
Expand All @@ -144,10 +149,11 @@ public function testInvalidTokenInHeader($method)
*/
public function testValidTokenRequestData($method)
{
$_SERVER['REQUEST_METHOD'] = $method;

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request([
'environment' => [
'REQUEST_METHOD' => $method,
],
'post' => ['_csrfToken' => 'testing123'],
'cookies' => ['csrfToken' => 'testing123']
]);
Expand All @@ -168,10 +174,11 @@ public function testValidTokenRequestData($method)
*/
public function testInvalidTokenRequestData($method)
{
$_SERVER['REQUEST_METHOD'] = $method;

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request([
'environment' => [
'REQUEST_METHOD' => $method,
],
'post' => ['_csrfToken' => 'nope'],
'cookies' => ['csrfToken' => 'testing123']
]);
Expand All @@ -189,10 +196,11 @@ public function testInvalidTokenRequestData($method)
*/
public function testInvalidTokenRequestDataMissing()
{
$_SERVER['REQUEST_METHOD'] = 'POST';

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request([
'environment' => [
'REQUEST_METHOD' => 'POST',
],
'post' => [],
'cookies' => ['csrfToken' => 'testing123']
]);
Expand All @@ -211,10 +219,11 @@ public function testInvalidTokenRequestDataMissing()
*/
public function testInvalidTokenMissingCookie($method)
{
$_SERVER['REQUEST_METHOD'] = $method;

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request([
'environment' => [
'REQUEST_METHOD' => $method
],
'post' => ['_csrfToken' => 'could-be-valid'],
'cookies' => []
]);
Expand All @@ -232,10 +241,9 @@ public function testInvalidTokenMissingCookie($method)
*/
public function testCsrfValidationSkipsRequestAction()
{
$_SERVER['REQUEST_METHOD'] = 'POST';

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request([
'environment' => ['REQUEST_METHOD' => 'POST'],
'params' => ['requested' => 1],
'post' => ['_csrfToken' => 'nope'],
'cookies' => ['csrfToken' => 'testing123']
Expand All @@ -256,10 +264,11 @@ public function testCsrfValidationSkipsRequestAction()
*/
public function testConfigurationCookieCreate()
{
$_SERVER['REQUEST_METHOD'] = 'GET';

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request(['webroot' => '/dir/']);
$controller->request = new Request([
'environment' => ['REQUEST_METHOD' => 'GET'],
'webroot' => '/dir/'
]);
$controller->response = new Response();

$component = new CsrfComponent($this->registry, [
Expand Down Expand Up @@ -290,10 +299,9 @@ public function testConfigurationCookieCreate()
*/
public function testConfigurationValidate()
{
$_SERVER['REQUEST_METHOD'] = 'POST';

$controller = $this->getMock('Cake\Controller\Controller', ['redirect']);
$controller->request = new Request([
'environment' => ['REQUEST_METHOD' => 'POST'],
'cookies' => ['csrfToken' => 'nope', 'token' => 'yes'],
'post' => ['_csrfToken' => 'no match', 'token' => 'yes'],
]);
Expand Down

0 comments on commit 0f818a2

Please sign in to comment.