Skip to content

Commit

Permalink
Browse files Browse the repository at this point in the history
Improved CSRF protection
  • Loading branch information
rskrzypczak committed Jan 18, 2022
1 parent 03f1688 commit 298c787
Show file tree
Hide file tree
Showing 10 changed files with 23 additions and 39 deletions.
4 changes: 1 addition & 3 deletions app/Controller/Action.php
Expand Up @@ -7,6 +7,7 @@
* @copyright YetiForce Sp. z o.o
* @license YetiForce Public License 4.0 (licenses/LicenseEN.txt or yetiforce.com)
* @author Mariusz Krzaczkowski <m.krzaczkowski@yetiforce.com>
* @author Radosław Skrzypczak <r.skrzypczak@yetiforce.com>
*/

namespace App\Controller;
Expand All @@ -16,9 +17,6 @@
*/
abstract class Action extends Base
{
/** {@inheritdoc} */
public $csrfActive = false;

/**
* Process action.
*
Expand Down
25 changes: 4 additions & 21 deletions app/Controller/Base.php
Expand Up @@ -7,6 +7,7 @@
* @copyright YetiForce Sp. z o.o
* @license YetiForce Public License 4.0 (licenses/LicenseEN.txt or yetiforce.com)
* @author Mariusz Krzaczkowski <m.krzaczkowski@yetiforce.com>
* @author Radosław Skrzypczak <r.skrzypczak@yetiforce.com>
*/

namespace App\Controller;
Expand All @@ -18,25 +19,13 @@ abstract class Base
{
/** @var \App\Headers Headers instance. */
public $headers;
/**
* CSRF is active?.
*
* @var bool
*/
public $csrfActive = true;

/**
* Activated language locale.
*
* @var bool
*/
protected static $activatedLocale = false;
/**
* Activated csrf.
*
* @var bool
*/
protected static $activatedCsrf = false;

/**
* Constructor.
Expand All @@ -48,15 +37,9 @@ public function __construct()
\App\Language::initLocale();
self::$activatedLocale = true;
}
if (!self::$activatedCsrf) {
if ($this->csrfActive && \App\Config::security('csrfActive')) {
require_once 'config/csrf_config.php';
\CsrfMagic\Csrf::init();
$this->csrfActive = true;
} else {
$this->csrfActive = false;
}
self::$activatedCsrf = true;
if (\App\Config::security('csrfActive')) {
require_once 'config/csrf_config.php';
\CsrfMagic\Csrf::init();
}
}

Expand Down
4 changes: 2 additions & 2 deletions app/Request.php
Expand Up @@ -710,8 +710,8 @@ public function validateWriteAccess($skipRequestTypeCheck = false)
throw new \App\Exceptions\Csrf('Invalid request - validate Write Access');
}
$this->validateReadAccess();
if (class_exists('CSRFConfig') && !\CsrfMagic\Csrf::check(false)) {
throw new \App\Exceptions\Csrf('Unsupported request');
if (\App\Config::security('csrfActive')) {
\CsrfMagic\Csrf::check();
}
}

Expand Down
6 changes: 6 additions & 0 deletions config/ConfigTemplates.php
Expand Up @@ -1154,6 +1154,12 @@
'validation' => '\App\Validator::bool',
'sanitization' => '\App\Purifier::bool'
],
'csrfLifetimeToken' => [
'default' => 28800,
'description' => 'Default expire time of CSRF token in seconds',
'validation' => '\App\Validator::naturalNumber',
'sanitization' => '\App\Purifier::naturalNumber'
],
'csrfFrameBreaker' => [
'default' => true,
'description' => 'Enable verified frame protection, used in CSRF',
Expand Down
3 changes: 3 additions & 0 deletions config/Security.php
Expand Up @@ -151,6 +151,9 @@ class Security
/** Enable CSRF protection */
public static $csrfActive = true;

/** Default expire time of CSRF token in seconds */
public static $csrfLifetimeToken = 28800;

/** Enable verified frame protection, used in CSRF */
public static $csrfFrameBreaker = true;

Expand Down
6 changes: 4 additions & 2 deletions config/csrf_config.php
Expand Up @@ -6,6 +6,7 @@
* The Initial Developer of the Original Code is vtiger.
* Portions created by vtiger are Copyright (C) vtiger.
* All Rights Reserved.
* Contributor(s): YetiForce Sp. z o.o
* ****************************************************************************** */

class CSRFConfig
Expand All @@ -16,14 +17,15 @@ class CSRFConfig
public static function startup()
{
//Override the default expire time of token
\CsrfMagic\Csrf::$expires = 259200;
\CsrfMagic\Csrf::$expires = \App\Config::security('csrfLifetimeToken', 7200);
\CsrfMagic\Csrf::$callback = function ($tokens) {
throw new \App\Exceptions\AppException('Invalid request - Response For Illegal Access', 403);
throw new \App\Exceptions\Csrf('Invalid request - Response For Illegal Access', 403);
};
$js = 'vendor/yetiforce/csrf-magic/src/Csrf.min.js';
if (!IS_PUBLIC_DIR) {
$js = 'public_html/' . $js;
}
\CsrfMagic\Csrf::$defer = true;
\CsrfMagic\Csrf::$dirSecret = __DIR__;
\CsrfMagic\Csrf::$rewriteJs = $js;
\CsrfMagic\Csrf::$cspToken = \App\Session::get('CSP_TOKEN');
Expand Down
4 changes: 2 additions & 2 deletions config/version.php
@@ -1,7 +1,7 @@
<?php

return [
'appVersion' => '6.3.42',
'patchVersion' => '2022.01.17',
'appVersion' => '6.3.43',
'patchVersion' => '2022.01.18',
'lib_roundcube' => '0.2.10',
];
4 changes: 1 addition & 3 deletions include/main/WebUI.php
Expand Up @@ -168,9 +168,7 @@ public function process(App\Request $request)
\App\Log::error("HandlerClass: $handlerClass", 'Loader');
throw new \App\Exceptions\AppException('LBL_HANDLER_NOT_FOUND', 405);
}
if ($handler->csrfActive) {
$handler->validateRequest($request);
}
$handler->validateRequest($request);
if ($handler->loginRequired() && $this->checkLogin($request)) {
return true;
}
Expand Down
3 changes: 0 additions & 3 deletions install/views/Index.php
Expand Up @@ -14,9 +14,6 @@ class Install_Index_View extends \App\Controller\View\Base
{
use \App\Controller\ExposeMethod;

/** {@inheritdoc} */
public $csrfActive = false;

/**
* @var bool
*/
Expand Down
3 changes: 0 additions & 3 deletions modules/Users/views/Login.php
Expand Up @@ -11,9 +11,6 @@

class Users_Login_View extends \App\Controller\View\Base
{
/** {@inheritdoc} */
public $csrfActive = false;

/** {@inheritdoc} */
public function __construct()
{
Expand Down

0 comments on commit 298c787

Please sign in to comment.