From 6e1beda538910b35f0f904357f5f2af78d798ec9 Mon Sep 17 00:00:00 2001 From: Robert Paprocki Date: Mon, 5 Feb 2018 16:27:32 -0800 Subject: [PATCH 1/2] Use constant time string comparison in FormKey validator CSRF tokens should be considered sensitive strings. While the risk of a malicious actor attempting gleam the form key via a timing attack is very low, we should still follow best practices in verifying this token. --- .../Magento/Framework/Data/Form/FormKey/Validator.php | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/lib/internal/Magento/Framework/Data/Form/FormKey/Validator.php b/lib/internal/Magento/Framework/Data/Form/FormKey/Validator.php index 0dbc9c879462e..99ae484977bfc 100644 --- a/lib/internal/Magento/Framework/Data/Form/FormKey/Validator.php +++ b/lib/internal/Magento/Framework/Data/Form/FormKey/Validator.php @@ -5,6 +5,8 @@ */ namespace Magento\Framework\Data\Form\FormKey; +use Magento\Framework\Encryption\Helper\Security; + /** * @api */ @@ -32,9 +34,11 @@ public function __construct(\Magento\Framework\Data\Form\FormKey $formKey) public function validate(\Magento\Framework\App\RequestInterface $request) { $formKey = $request->getParam('form_key', null); - if (!$formKey || $formKey !== $this->_formKey->getFormKey()) { + + if (!$formKey) { return false; } - return true; + + return Security::compareStrings($formKey, $this->_formKey->getFormKey()); } } From 11a95d641009dbe57a66fc2284814e9a9920502f Mon Sep 17 00:00:00 2001 From: Vlad Veselov Date: Tue, 26 Jun 2018 11:24:12 +0300 Subject: [PATCH 2/2] Polish up implementation --- .../Magento/Framework/Data/Form/FormKey/Validator.php | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/lib/internal/Magento/Framework/Data/Form/FormKey/Validator.php b/lib/internal/Magento/Framework/Data/Form/FormKey/Validator.php index 99ae484977bfc..225ff1fd140a9 100644 --- a/lib/internal/Magento/Framework/Data/Form/FormKey/Validator.php +++ b/lib/internal/Magento/Framework/Data/Form/FormKey/Validator.php @@ -34,11 +34,7 @@ public function __construct(\Magento\Framework\Data\Form\FormKey $formKey) public function validate(\Magento\Framework\App\RequestInterface $request) { $formKey = $request->getParam('form_key', null); - - if (!$formKey) { - return false; - } - - return Security::compareStrings($formKey, $this->_formKey->getFormKey()); + + return $formKey && Security::compareStrings($formKey, $this->_formKey->getFormKey()); } }