Skip to content
Permalink
Browse files Browse the repository at this point in the history
Implement CSRF token by default
Implement CSRF protection on CMS for postback handling
  • Loading branch information
daftspunk committed Oct 29, 2017
1 parent 08989ff commit 4a6e0e1
Show file tree
Hide file tree
Showing 2 changed files with 30 additions and 1 deletion.
2 changes: 1 addition & 1 deletion config/cms.php
Expand Up @@ -332,7 +332,7 @@
|
*/

'enableCsrfProtection' => false,
'enableCsrfProtection' => true,

/*
|--------------------------------------------------------------------------
Expand Down
29 changes: 29 additions & 0 deletions modules/cms/classes/Controller.php
Expand Up @@ -337,6 +337,7 @@ public function runPage($page, $useAjax = true)
if (
$useAjax &&
($handler = post('_handler')) &&
($this->verifyCsrfToken()) &&
($handlerResponse = $this->runAjaxHandler($handler)) &&
$handlerResponse !== true
) {
Expand Down Expand Up @@ -1355,4 +1356,32 @@ protected function setComponentPropertiesFromParams($component, $parameters = []
}
}
}

//
// Security
//

/**
* Checks the request data / headers for a valid CSRF token.
* Returns false if a valid token is not found. Override this
* method to disable the check.
* @return bool
*/
protected function verifyCsrfToken()
{
if (!Config::get('cms.enableCsrfProtection')) {
return true;
}

if (in_array(Request::method(), ['HEAD', 'GET', 'OPTIONS'])) {
return true;
}

$token = Request::input('_token') ?: Request::header('X-CSRF-TOKEN');

return hash_equals(
Session::token(),
$token
);
}
}

5 comments on commit 4a6e0e1

@ZainSabahat
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can confirm that it has mitigated the CSRF vulnerability for postback handling.

@ARH-Digital
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@daftspunk and @ZainSabahat
Is it safe to assume that the CSRF vulnerability was present in most versions prior to build 426 as well?
Just seeking clarification that version 419 and earlier ,that cannot be upgraded due to PHP versions, need to be manually patched?

@LukeTowers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@ARH-Digital That would be correct, although you should really update your environments to use more modern versions of PHP to stay up to date with all of the security fixes, performance improvements, general bug fixes, and features to come. A host that provides >= PHP7 is not expensive, there is rarely a good reason to stick with <7, especially when factoring in the performance improvements in 7

@ARH-Digital
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the confirmation. I totally agree with the hosting. However, I work for an agency and we do offer hosting, but alas, we cannot control the environments of all our clients, and are restricted in some instances.

@LukeTowers
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let your clients know what they risk by not upgrading versions and perhaps provide them options for better hosts that will upgrade if their current one refuses to.

Please sign in to comment.