[TASK] Use hash_equals for timing-safe comparison of hash-values 98/55098/2
authorStefan Neufeind <typo3.neufeind@speedpartner.de>
Fri, 15 Dec 2017 16:18:27 +0000 (17:18 +0100)
committerStefan Neufeind <typo3.neufeind@speedpartner.de>
Tue, 13 Feb 2018 07:56:07 +0000 (08:56 +0100)
To prevent timing-attacks on hash-comparions it is advised
to use hash_equals.

Resolves: #83329
Releases: master, 8.7
Change-Id: I7539ed27538d7d81767bfce582d568cff09d1610
Reviewed-on: https://review.typo3.org/55098
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Oliver Klee <typo3-coding@oliverklee.de>
Reviewed-by: Alexander Opitz <opitz.alexander@googlemail.com>
Reviewed-by: Stephan GroƟberndt <stephan@grossberndt.de>
Tested-by: Alexander Opitz <opitz.alexander@googlemail.com>
Reviewed-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
Tested-by: Stefan Neufeind <typo3.neufeind@speedpartner.de>
13 files changed:
typo3/sysext/backend/Classes/Controller/FileSystemNavigationFrameController.php
typo3/sysext/backend/Classes/Controller/FormInlineAjaxController.php
typo3/sysext/backend/Classes/Controller/LinkBrowserController.php
typo3/sysext/backend/Classes/Controller/Wizard/ColorpickerController.php
typo3/sysext/backend/Classes/Form/Wizard/ImageManipulationWizard.php
typo3/sysext/core/Classes/Controller/FileDumpController.php
typo3/sysext/core/Classes/FormProtection/AbstractFormProtection.php
typo3/sysext/extbase/Classes/Security/Cryptography/HashService.php
typo3/sysext/frontend/Classes/Controller/ShowImageController.php
typo3/sysext/frontend/Classes/Controller/TypoScriptFrontendController.php
typo3/sysext/saltedpasswords/Classes/Salt/Pbkdf2Salt.php
typo3/sysext/saltedpasswords/Classes/Salt/PhpassSalt.php
typo3/sysext/saltedpasswords/Classes/SaltedPasswordService.php

index ee1311c..b1992ca 100644 (file)
@@ -102,7 +102,7 @@ class FileSystemNavigationFrameController
         $scopeData = (string)GeneralUtility::_GP('scopeData');
         $scopeHash = (string)GeneralUtility::_GP('scopeHash');
 
-        if (!empty($scopeData) && GeneralUtility::hmac($scopeData) === $scopeHash) {
+        if (!empty($scopeData) && hash_equals(GeneralUtility::hmac($scopeData), $scopeHash)) {
             $this->scopeData = unserialize($scopeData);
         }
 
index d2d2eba..3fbbd0b 100644 (file)
@@ -771,7 +771,7 @@ class FormInlineAjaxController extends AbstractFormEngineAjaxController
         if (empty($context['config'])) {
             throw new \RuntimeException('Empty context config section given', 1489751362);
         }
-        if (!\hash_equals(GeneralUtility::hmac(json_encode($context['config']), 'InlineContext'), $context['hmac'])) {
+        if (!hash_equals(GeneralUtility::hmac(json_encode($context['config']), 'InlineContext'), $context['hmac'])) {
             throw new \RuntimeException('Hash does not validate', 1489751363);
         }
         return $context['config'];
index 8b28d15..e2151fd 100644 (file)
@@ -122,7 +122,7 @@ class LinkBrowserController extends AbstractLinkBrowserController
                 }
                 unset($value);
             }
-            $result = $this->parameters['fieldChangeFuncHash'] === GeneralUtility::hmac(serialize($fieldChangeFunctions));
+            $result = hash_equals(GeneralUtility::hmac(serialize($fieldChangeFunctions)), $this->parameters['fieldChangeFuncHash']);
         }
         return $result;
     }
index 91cb1e9..c9dcf84 100644 (file)
@@ -411,7 +411,7 @@ class ColorpickerController extends AbstractWizardController
      */
     protected function areFieldChangeFunctionsValid()
     {
-        return $this->fieldChangeFunc && $this->fieldChangeFuncHash && $this->fieldChangeFuncHash === GeneralUtility::hmac($this->fieldChangeFunc);
+        return $this->fieldChangeFunc && $this->fieldChangeFuncHash && hash_equals(GeneralUtility::hmac($this->fieldChangeFunc), $this->fieldChangeFuncHash);
     }
 
     /**
index 0c0d150..405606f 100644 (file)
@@ -87,6 +87,6 @@ class ImageManipulationWizard
     protected function isSignatureValid(ServerRequestInterface $request)
     {
         $token = GeneralUtility::hmac($request->getQueryParams()['arguments'], 'ajax_wizard_image_manipulation');
-        return $token === $request->getQueryParams()['signature'];
+        return hash_equals($token, $request->getQueryParams()['signature']);
     }
 }
index c0a367d..538c3a2 100644 (file)
@@ -55,7 +55,7 @@ class FileDumpController
             $parameters['p'] = $p;
         }
 
-        if (GeneralUtility::hmac(implode('|', $parameters), 'resourceStorageDumpFile') === $this->getGetOrPost($request, 'token')) {
+        if (hash_equals(GeneralUtility::hmac(implode('|', $parameters), 'resourceStorageDumpFile'), $this->getGetOrPost($request, 'token'))) {
             if (isset($parameters['f'])) {
                 try {
                     $file = ResourceFactory::getInstance()->getFileObject($parameters['f']);
index dc634da..a49cc6d 100644 (file)
@@ -103,7 +103,7 @@ abstract class AbstractFormProtection
     public function validateToken($tokenId, $formName, $action = '', $formInstanceName = '')
     {
         $validTokenId = GeneralUtility::hmac(((string)$formName . (string)$action) . (string)$formInstanceName . $this->getSessionToken());
-        if ((string)$tokenId === $validTokenId) {
+        if (hash_equals($validTokenId, (string)$tokenId)) {
             $isValid = true;
         } else {
             $isValid = false;
index 8426306..b5af431 100644 (file)
@@ -65,7 +65,7 @@ class HashService implements \TYPO3\CMS\Core\SingletonInterface
      */
     public function validateHmac($string, $hmac)
     {
-        return $this->generateHmac($string) === $hmac;
+        return hash_equals($this->generateHmac($string), $hmac);
     }
 
     /**
index 62748b0..f518ccb 100644 (file)
@@ -119,7 +119,7 @@ EOF;
         /* For backwards compatibility the HMAC is transported within the md5 param */
         $hmacParameter = isset($this->request->getQueryParams()['md5']) ? $this->request->getQueryParams()['md5'] : null;
         $hmac = GeneralUtility::hmac(implode('|', [$fileUid, $parametersEncoded]));
-        if ($hmac !== $hmacParameter) {
+        if (!hash_equals($hmac, $hmacParameter)) {
             throw new \InvalidArgumentException('hash does not match', 1476048456);
         }
 
index 6659137..71caa8a 100644 (file)
@@ -2181,7 +2181,7 @@ class TypoScriptFrontendController
             $GET['id'] = $this->id;
             $this->cHash_array = $this->cacheHash->getRelevantParameters(GeneralUtility::implodeArrayForUrl('', $GET));
             $cHash_calc = $this->cacheHash->calculateCacheHash($this->cHash_array);
-            if ($cHash_calc != $this->cHash) {
+            if (!hash_equals($cHash_calc, $this->cHash)) {
                 if ($GLOBALS['TYPO3_CONF_VARS']['FE']['pageNotFoundOnCHashError']) {
                     $this->pageNotFoundAndExit('Request parameters could not be validated (&cHash comparison failed)');
                 } else {
index 5578472..fdb2534 100644 (file)
@@ -108,7 +108,7 @@ class Pbkdf2Salt extends AbstractSalt implements SaltInterface
      */
     public function checkPassword($plainPW, $saltedHashPW)
     {
-        return $this->isValidSalt($saltedHashPW) && \hash_equals($this->getHashedPassword($plainPW, $saltedHashPW), $saltedHashPW);
+        return $this->isValidSalt($saltedHashPW) && hash_equals($this->getHashedPassword($plainPW, $saltedHashPW), $saltedHashPW);
     }
 
     /**
index 89d93cb..ddff29c 100644 (file)
@@ -125,7 +125,7 @@ class PhpassSalt extends AbstractSalt implements SaltInterface
     public function checkPassword($plainPW, $saltedHashPW)
     {
         $hash = $this->cryptPassword($plainPW, $saltedHashPW);
-        return $hash && \hash_equals($hash, $saltedHashPW);
+        return $hash && hash_equals($hash, $saltedHashPW);
     }
 
     /**
index 63cfd6d..9a7b7df 100644 (file)
@@ -135,13 +135,13 @@ class SaltedPasswordService extends \TYPO3\CMS\Sv\AbstractAuthenticationService
                     $this->authenticationFailed = true;
                 }
             } elseif (preg_match('/[0-9abcdef]{32,32}/', $user['password'])) {
-                $validPasswd = \hash_equals(md5($password), (string)$user['password']);
+                $validPasswd = hash_equals(md5($password), (string)$user['password']);
                 // Skip further authentication methods
                 if (!$validPasswd) {
                     $this->authenticationFailed = true;
                 }
             } else {
-                $validPasswd = (string)$password !== '' && \hash_equals((string)$user['password'], (string)$password);
+                $validPasswd = (string)$password !== '' && hash_equals((string)$user['password'], (string)$password);
             }
             // Should we store the new format value in DB?
             if ($validPasswd && (int)$this->extConf['updatePasswd']) {