[TASK] Use hash_equals for timing-safe comparison of hash-values 74/55074/5
authorStefan Neufeind <typo3.neufeind@speedpartner.de>
Thu, 14 Dec 2017 13:51:32 +0000 (14:51 +0100)
committerHelmut Hummel <typo3@helhum.io>
Fri, 15 Dec 2017 15:22:18 +0000 (16:22 +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/55074
Tested-by: TYPO3com <no-reply@typo3.com>
Reviewed-by: Susanne Moog <susanne.moog@typo3.org>
Tested-by: Susanne Moog <susanne.moog@typo3.org>
Reviewed-by: Stephan GroƟberndt <stephan@grossberndt.de>
Reviewed-by: Helmut Hummel <typo3@helhum.io>
Tested-by: Helmut Hummel <typo3@helhum.io>
12 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/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 85eab22..5b069f7 100644 (file)
@@ -103,7 +103,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 00ef626..ef8054b 100644 (file)
@@ -743,7 +743,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 195addb..a669518 100644 (file)
@@ -120,7 +120,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 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 ebaa9f6..dcd25e7 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 b53b405..f9912cd 100644 (file)
@@ -120,7 +120,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 fb83d98..a34872b 100644 (file)
@@ -2133,7 +2133,7 @@ class TypoScriptFrontendController implements LoggerAwareInterface
             $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 ab80faa..04fd93b 100644 (file)
@@ -109,7 +109,7 @@ class Pbkdf2Salt extends AbstractComposedSalt
      */
     public function checkPassword(string $plainPW, string $saltedHashPW): bool
     {
-        return $this->isValidSalt($saltedHashPW) && \hash_equals($this->getHashedPassword($plainPW, $saltedHashPW), $saltedHashPW);
+        return $this->isValidSalt($saltedHashPW) && hash_equals($this->getHashedPassword($plainPW, $saltedHashPW), $saltedHashPW);
     }
 
     /**
index 9dcf084..f2553d6 100644 (file)
@@ -126,7 +126,7 @@ class PhpassSalt extends AbstractComposedSalt
     public function checkPassword(string $plainPW, string $saltedHashPW): bool
     {
         $hash = $this->cryptPassword($plainPW, $saltedHashPW);
-        return $hash && \hash_equals($hash, $saltedHashPW);
+        return $hash && hash_equals($hash, $saltedHashPW);
     }
 
     /**
index e8d1e49..4cc6fec 100644 (file)
@@ -137,13 +137,13 @@ class SaltedPasswordService extends 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']) {