From 4585051505e2c887cb51e886f79aa1848f7aa013 Mon Sep 17 00:00:00 2001 From: Ali Alam Date: Thu, 13 Feb 2025 13:55:17 +0800 Subject: [PATCH] System: add nonce and CSRF token handling to all POST forms (#1891) Co-authored-by: Sandra Kuipers --- gibbon.php | 18 ++++- src/Data/Validator.php | 11 +-- src/Forms/Form.php | 11 ++- src/Services/CoreServiceProvider.php | 6 +- src/Session/Session.php | 14 +--- src/Session/TokenHandler.php | 108 ++++++++++++++++++++++++++ src/View/Components/ReturnMessage.php | 1 + tests/unit/Forms/FormTest.php | 11 ++- 8 files changed, 145 insertions(+), 35 deletions(-) create mode 100644 src/Session/TokenHandler.php diff --git a/gibbon.php b/gibbon.php index a758bcab48..1f282b0ef4 100644 --- a/gibbon.php +++ b/gibbon.php @@ -20,6 +20,8 @@ */ use Gibbon\Http\Url; +use Gibbon\Data\Validator; +use Gibbon\Session\TokenHandler; // Handle fatal errors more gracefully register_shutdown_function(function () { @@ -144,16 +146,26 @@ } // Sanitize incoming user-supplied GET variables -$validator = $container->get(\Gibbon\Data\Validator::class); +$validator = $container->get(Validator::class); $_GET = $validator->sanitizeUrlParams($_GET); +$tokenHandler = $container->get(TokenHandler::class); -// Check for CSRF token when posting any form +// Check for CSRF token and nonce when posting any form if (!empty($_POST) && stripos($_SERVER['PHP_SELF'], 'Process.php') !== false) { - if (!$validator->validateToken()) { + + // Validate CSRF token + if (!$tokenHandler->validateCsrfToken()) { $URL .= $_SERVER['HTTP_REFERER'].'&return=error9'; header("Location: {$URL}"); exit; } + + // Validate nonce + if (!$tokenHandler->validateNonce()) { + $URL .= $_SERVER['HTTP_REFERER'].'&return=error10'; + header("Location: {$URL}"); + exit; + } } diff --git a/src/Data/Validator.php b/src/Data/Validator.php index 1bb9121ed6..5f10f129ce 100644 --- a/src/Data/Validator.php +++ b/src/Data/Validator.php @@ -30,17 +30,15 @@ */ class Validator { - protected $csrfToken; protected $allowableHTML; protected $allowableHTMLString; protected $allowableIframeSources; - public function __construct(string $allowableHTMLString, string $allowableIframeSources = '', string $csrfToken = '') + public function __construct(string $allowableHTMLString, string $allowableIframeSources = '') { $this->allowableHTMLString = $allowableHTMLString; $this->allowableHTML = $this->parseTagsFromString($this->allowableHTMLString); $this->allowableIframeSources = explode(',', mb_strtolower($allowableIframeSources)); - $this->csrfToken = $csrfToken; } public function getAllowableHTML() @@ -52,12 +50,7 @@ public function getAllowableIframeSources() { return $this->allowableIframeSources; } - - public function validateToken() - { - return !empty($this->csrfToken) && $this->csrfToken === $_POST['token']; - } - + /** * Sanitize the input data. * diff --git a/src/Forms/Form.php b/src/Forms/Form.php index c23ab16872..bf8ca39ae5 100644 --- a/src/Forms/Form.php +++ b/src/Forms/Form.php @@ -23,12 +23,13 @@ use Gibbon\Http\Url; use Gibbon\Tables\Action; +use Gibbon\Session\TokenHandler; use Gibbon\Forms\View\FormBlankView; use Gibbon\Forms\View\FormTableView; use Gibbon\Forms\FormFactoryInterface; +use League\Container\ContainerAwareTrait; use Gibbon\Forms\View\FormRendererInterface; use Gibbon\Forms\Traits\BasicAttributesTrait; -use League\Container\ContainerAwareTrait; /** * Form @@ -91,9 +92,11 @@ public static function create($id, $action, $method = 'post', $class = 'standard ->setAction($action) ->setMethod($method); - // Add the CSRF token to all forms - $form->addHiddenValue('token', $container->has('token') ? $container->get('token') : ''); - + // Add the CSRF and Nonce tokens to all forms + $tokenHandler = $container->get(TokenHandler::class); + $form->addHiddenValue('token', $tokenHandler->getCSRF()); + $form->addHiddenValue('nonce', $tokenHandler->getNonce()); + // Enable quick save by default on edit and settings pages if ($form->checkActionList($action, ['settingsProcess', 'editProcess'])) { $form->enableQuickSave(); diff --git a/src/Services/CoreServiceProvider.php b/src/Services/CoreServiceProvider.php index 19a57cb5c6..b378c71527 100644 --- a/src/Services/CoreServiceProvider.php +++ b/src/Services/CoreServiceProvider.php @@ -257,10 +257,6 @@ public function register() return $page; }); - $container->share('token', function () { - return $this->getLeagueContainer()->get('session')->get('token'); - }); - $container->add(MailerInterface::class, function () use ($container) { $view = new View($container->get('twig')); return (new Mailer($container->get('session')))->setView($view); @@ -287,7 +283,7 @@ public function register() $container->share(Validator::class, function () { $session = $this->getLeagueContainer()->get('session'); - return new Validator($session->get('allowableHTML', ''), $session->get('allowableIframeSources', ''), $session->get('token', '')); + return new Validator($session->get('allowableHTML', ''), $session->get('allowableIframeSources', '')); }); $container->add(PasswordPolicy::class, function () use ($container) { diff --git a/src/Session/Session.php b/src/Session/Session.php index 8393a91fec..181960affc 100644 --- a/src/Session/Session.php +++ b/src/Session/Session.php @@ -49,12 +49,7 @@ class Session implements SessionInterface * @param string $action * Optional string of the current action. */ - public function __construct( - string $guid, - string $address = '', - string $module = '', - string $action = '' - ) + public function __construct(string $guid, string $address = '', string $module = '', string $action = '') { // Backwards compatibility for external modules. $this->guid = $guid; @@ -64,13 +59,8 @@ public function __construct( $this->set('address', $address); $this->set('module', $module); $this->set('action', $action); - - // Create a CSRF token - if (!$this->exists('token')) { - $this->set('token', bin2hex(random_bytes(16))); - } } - + public function setGuid(string $_guid) { $this->guid = $_guid; diff --git a/src/Session/TokenHandler.php b/src/Session/TokenHandler.php new file mode 100644 index 0000000000..117b3ca0d5 --- /dev/null +++ b/src/Session/TokenHandler.php @@ -0,0 +1,108 @@ +. +*/ + +namespace Gibbon\Session; + +use Gibbon\Contracts\Services\Session as SessionInterface; + +/** + * tokenHandler Class + * + * @version v29 + * @since v29 + */ + + class TokenHandler { + + /** + * @var SessionInterface + */ + private $session; + + public function __construct(SessionInterface $session) + { + $this->session = $session; + + // Create a CSRF token + if (!$session->exists('token')) { + $session->set('token', bin2hex(random_bytes(16))); + } + + // Create a nonce list + if (!$session->exists('nonceList')) { + $session->set('nonceList', []); + } + } + + public function getCSRF() + { + return $this->session->get('token'); + } + + public function validateCsrfToken() + { + return !empty($this->session->get('token')) && $this->session->get('token') === $_POST['token']; + } + + + public function validateNonce() + { + $nonce = $_POST['nonce'] ?? ''; + + if(empty($nonce)) { + return false; + } + + return $this->removeNonce($nonce); + } + + /** + * Create and add a nonce to the session's nonce list, then return it. + */ + public function getNonce() + { + $nonce = bin2hex(random_bytes(16)); + + $nonceList = $this->session->get('nonceList', []); + $nonceList[] = $nonce; + $this->session->set('nonceList', $nonceList); + + return $nonce; + } + + /** + * Check if a nonce exists when a form is submitted. If yes, then remove it from the list. + * + * @param string $nonce + * @return bool + */ + public function removeNonce(string $nonce) { + $nonceList = $this->session->get('nonceList', []); + + if (($key = array_search($nonce, $nonceList)) !== false) { + unset($nonceList[$key]); + $this->session->set('nonceList', array_values($nonceList)); + return true; + } + return false; + } + + } diff --git a/src/View/Components/ReturnMessage.php b/src/View/Components/ReturnMessage.php index b198a044a4..4e7c3e4174 100644 --- a/src/View/Components/ReturnMessage.php +++ b/src/View/Components/ReturnMessage.php @@ -55,6 +55,7 @@ public function __construct() 'error7' => __('Your request failed because some required values were not unique.'), 'error8' => __('Your request failed because the link is invalid or has expired.'), 'error9' => __('Your request failed because your session authentication has expired. Please log out and log in again.'), + 'error10' => __('Your request failed because you are trying to re-submit a form that has been submitted before. Please submit a new form.'), //Warnings 'warning0' => __('Your optional extra data failed to save.'), diff --git a/tests/unit/Forms/FormTest.php b/tests/unit/Forms/FormTest.php index 9b1b71dfe4..26b6a34f21 100644 --- a/tests/unit/Forms/FormTest.php +++ b/tests/unit/Forms/FormTest.php @@ -13,9 +13,12 @@ use PHPUnit\Framework\TestCase; use Gibbon\Forms\View\FormRendererInterface; +use Gibbon\Session\TokenHandler; use Gibbon\Services\ViewServiceProvider; use League\Container\Container; use Gibbon\Forms\View\FormView; +use Gibbon\Contracts\Services\Session as SessionInterface; +use Gibbon\Session\Session; /** * @covers Form @@ -59,8 +62,12 @@ public function setUp(): void return $twig; }); - $container->share('token', 'test-token-value'); - + $container->share(SessionInterface::class, function () { + return new Session('test-guid'); + }); + $container->share(TokenHandler::class, function () use ($container) { + return new TokenHandler($container->get(SessionInterface::class)); + }); $service = new ViewServiceProvider(); $service->setContainer($container); $service->register();