[SECURITY] Protect arguments of form __referrer with HMAC
authorFelix Oertel <f@oer.tel>
Wed, 7 Mar 2012 10:55:58 +0000 (11:55 +0100)
committerFelix Oertel <f@oer.tel>
Wed, 28 Mar 2012 11:38:09 +0000 (13:38 +0200)
The request arguments of the referring request are
a serialized string written to one of the hidden
fields in a Fluid form. This string has to be protected
by a HMAC to protect Extbase from possible unserialize
attacks.

Note: For now there is no object known within Extbase,
that could be used for an unserialize exploit!

This change also backports some convenience hmac methods
to the hash service from the current master, to have the
bugfix in sync.

Also this backports some of the method renamings from
FLOW3.

Change-Id: Iaba9e7556c7aeeb8a3724c84d96160a511500e98
Security-Bulletin: TYPO3-CORE-SA-2012-001
Fixes: #35310
Releases: 1.4, 4.7, 6.0

typo3/sysext/extbase/Classes/MVC/Web/Request.php
typo3/sysext/extbase/Classes/Security/Cryptography/HashService.php
typo3/sysext/extbase/Classes/Security/Exception/InvalidHash.php [new file with mode: 0644]
typo3/sysext/extbase/Tests/Unit/MVC/Web/RequestTest.php
typo3/sysext/extbase/Tests/Unit/Security/Cryptography/HashServiceTest.php

index dd5fd8e..eac242f 100644 (file)
 class Tx_Extbase_MVC_Web_Request extends Tx_Extbase_MVC_Request {
 
        /**
+        * @var Tx_Extbase_Security_Cryptography_HashService
+        */
+       protected $hashService;
+
+       /**
         * @var string The requested representation format
         */
        protected $format = 'html';
@@ -74,6 +79,13 @@ class Tx_Extbase_MVC_Web_Request extends Tx_Extbase_MVC_Request {
        protected $configurationManager;
 
        /**
+        * @param Tx_Extbase_Security_Cryptography_HashService $hashService
+        */
+       public function injectHashService(Tx_Extbase_Security_Cryptography_HashService $hashService) {
+               $this->hashService = $hashService;
+       }
+
+       /**
         * @param Tx_Extbase_Configuration_ConfigurationManagerInterface $configurationManager
         * @return void
         */
@@ -214,7 +226,9 @@ class Tx_Extbase_MVC_Web_Request extends Tx_Extbase_MVC_Request {
 
                        $arguments = array();
                        if (isset($referrerArray['arguments'])) {
-                               $arguments = unserialize($referrerArray['arguments']);
+                               $serializedArgumentsWithHmac = $referrerArray['arguments'];
+                               $serializedArguments = $this->hashService->validateAndStripHmac($serializedArgumentsWithHmac);
+                               $arguments = unserialize(base64_decode($serializedArguments));
                                unset($referrerArray['arguments']);
                        }
 
index 2e4ca3b..e71e3b2 100644 (file)
  * @license http://www.gnu.org/licenses/lgpl.html GNU Lesser Public License, version 3 or later
  */
 class Tx_Extbase_Security_Cryptography_HashService implements t3lib_singleton {
+
        /**
         * Generate a hash for a given string
         *
         * @param string $string The string for which a hash should be generated
         * @return string The hash of the string
-        * @throws F3\FLOW3\Security\Exception\InvalidArgumentForHashGeneration if something else than a string was given as parameter
-        * @todo Mark as API once it is more stable
+        * @deprecated since Extbase 6.0, will be removed in Extbase 6.2
         */
        public function generateHash($string) {
+               t3lib_div::logDeprecatedFunction();
+               return $this->generateHmac($string);
+       }
+
+       /**
+        * Generate a hash (HMAC) for a given string
+        *
+        * @param string $string The string for which a hash should be generated
+        * @return string The hash of the string
+        * @throws Tx_Extbase_Security_Exception_InvalidArgumentForHashGeneration if something else than a string was given as parameter
+        */
+       public function generateHmac($string) {
                if (!is_string($string)) throw new Tx_Extbase_Security_Exception_InvalidArgumentForHashGeneration('A hash can only be generated for a string, but "' . gettype($string) . '" was given.', 1255069587);
                $encryptionKey = $GLOBALS['TYPO3_CONF_VARS']['SYS']['encryptionKey'];
                if (!$encryptionKey) throw new Tx_Extbase_Security_Exception_InvalidArgumentForHashGeneration('Encryption Key was empty!', 1255069597);
@@ -50,15 +62,67 @@ class Tx_Extbase_Security_Cryptography_HashService implements t3lib_singleton {
        }
 
        /**
+        * Appends a hash (HMAC) to a given string and returns the result
+        *
+        * @param string $string The string for which a hash should be generated
+        * @return string The original string with HMAC of the string appended
+        * @see generateHmac()
+        * @todo Mark as API once it is more stable
+        */
+       public function appendHmac($string) {
+               $hmac = $this->generateHmac($string);
+               return $string . $hmac;
+       }
+
+       /**
         * Test if a string $string has the hash given by $hash.
         *
         * @param string $string The string which should be validated
         * @param string $hash The hash of the string
         * @return boolean TRUE if string and hash fit together, FALSE otherwise.
-        * @todo Mark as API once it is more stable
+        * @deprecated since Extbase 6.0, will be removed in Extbase 6.2
         */
        public function validateHash($string, $hash) {
-               return ($this->generateHash($string) === $hash);
+               t3lib_div::logDeprecatedFunction();
+               return $this->validateHmac($string, $hash);
+       }
+
+       /**
+        * Tests if a string $string matches the HMAC given by $hash.
+        *
+        * @param string $string The string which should be validated
+        * @param string $hmac The hash of the string
+        * @return boolean TRUE if string and hash fit together, FALSE otherwise.
+        */
+       public function validateHmac($string, $hmac) {
+               return ($this->generateHmac($string) === $hmac);
+       }
+
+       /**
+        * Tests if the last 40 characters of a given string $string
+        * matches the HMAC of the rest of the string and, if true,
+        * returns the string without the HMAC. In case of a HMAC
+        * validation error, an exception is thrown.
+        *
+        * @param string $string The string with the HMAC appended (in the format 'string<HMAC>')
+        * @return string the original string without the HMAC, if validation was successful
+        * @see validateHmac()
+        * @throws Tx_Extbase_Security_Exception_InvalidArgumentForHashGeneration if the given string is not well-formatted
+        * @throws Tx_Extbase_Security_Exception_InvalidHash if the hash did not fit to the data.
+        * @todo Mark as API once it is more stable
+        */
+       public function validateAndStripHmac($string) {
+               if (!is_string($string)) {
+                       throw new Tx_Extbase_Security_Exception_InvalidArgumentForHashGeneration('A hash can only be validated for a string, but "' . gettype($string) . '" was given.', 1320829762);
+               }
+               if (strlen($string) < 40) {
+                       throw new Tx_Extbase_Security_Exception_InvalidArgumentForHashGeneration('A hashed string must contain at least 40 characters, the given string was only ' . strlen($string) . ' characters long.', 1320830276);
+               }
+               $stringWithoutHmac = substr($string, 0, -40);
+               if ($this->validateHmac($stringWithoutHmac, substr($string, -40)) !== TRUE) {
+                       throw new Tx_Extbase_Security_Exception_InvalidHash('The given string was not appended with a valid HMAC.', 1320830018);
+               }
+               return $stringWithoutHmac;
        }
 }
 ?>
\ No newline at end of file
diff --git a/typo3/sysext/extbase/Classes/Security/Exception/InvalidHash.php b/typo3/sysext/extbase/Classes/Security/Exception/InvalidHash.php
new file mode 100644 (file)
index 0000000..b63911e
--- /dev/null
@@ -0,0 +1,36 @@
+<?php
+/***************************************************************
+*  Copyright notice
+*
+*  (c) 2012 Extbase Team
+*  All rights reserved
+*
+*  This class is a backport of the corresponding class of FLOW3.
+*  All credits go to the v5 team.
+*
+*  This script is part of the TYPO3 project. The TYPO3 project is
+*  free software; you can redistribute it and/or modify
+*  it under the terms of the GNU General Public License as published by
+*  the Free Software Foundation; either version 2 of the License, or
+*  (at your option) any later version.
+*
+*  The GNU General Public License can be found at
+*  http://www.gnu.org/copyleft/gpl.html.
+*
+*  This script is distributed in the hope that it will be useful,
+*  but WITHOUT ANY WARRANTY; without even the implied warranty of
+*  MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+*  GNU General Public License for more details.
+*
+*  This copyright notice MUST APPEAR in all copies of the script!
+***************************************************************/
+
+/**
+ * A "InvalidHash" Exception, thrown when a HMAC validation failed.
+ *
+ * @package Extbase
+ * @subpackage Security\Exception
+ */
+class Tx_Extbase_Security_Exception_InvalidHash extends Tx_Extbase_Security_Exception {
+}
+?>
\ No newline at end of file
index 0d69dd0..a4d4081 100644 (file)
@@ -66,5 +66,20 @@ class Tx_Extbase_Tests_Unit_MVC_Web_RequestTest extends Tx_Extbase_Tests_Unit_Ba
                $this->assertAttributeEquals('Foo', 'controllerName', $referringRequest);
                $this->assertAttributeEquals('bar', 'controllerActionName', $referringRequest);
        }
+
+       /**
+        * @test
+        * @expectedException \TYPO3\FLOW3\Security\Exception\InvalidHashException
+        */
+       public function getReferringRequestThrowsAnExceptionIfTheHmacOfTheArgumentsCouldNotBeValidated() {
+               $request = $this->getAccessibleMock('TYPO3\FLOW3\MVC\Web\Request', array('dummy'));
+               $request->setArgument('__referrer', array(
+                       '@controller' => 'Foo',
+                       '@action' => 'bar',
+                       'arguments' => base64_encode('some manipulated arguments string without valid HMAC')
+               ));
+               $request->_set('hashService', new \TYPO3\FLOW3\Security\Cryptography\HashService());
+               $request->getReferringRequest();
+       }
 }
 ?>
\ No newline at end of file
index 06d8f7c..9180faf 100644 (file)
@@ -92,5 +92,64 @@ class Tx_Extbase_Tests_Unit_Security_Cryptography_HashServiceTest extends Tx_Ext
                $hash = 'myhash';
                $this->assertFalse($this->hashService->validateHash($string, $hash));
        }
+
+       /**
+        * @test
+        * @expectedException TYPO3\FLOW3\Security\Exception\InvalidArgumentForHashGenerationException
+        */
+       public function appendHmacThrowsExceptionIfNoStringGiven() {
+               $this->hashService->appendHmac(NULL);
+       }
+
+       /**
+        * @test
+        */
+       public function appendHmacAppendsHmacToGivenString() {
+               $string = 'This is some arbitrary string ';
+               $hashedString = $this->hashService->appendHmac($string);
+               $this->assertSame($string, substr($hashedString, 0, -40));
+       }
+
+       /**
+        * @test
+        * @expectedException TYPO3\FLOW3\Security\Exception\InvalidArgumentForHashGenerationException
+        */
+       public function validateAndStripHmacThrowsExceptionIfNoStringGiven() {
+               $this->hashService->validateAndStripHmac(NULL);
+       }
+
+       /**
+        * @test
+        * @expectedException TYPO3\FLOW3\Security\Exception\InvalidArgumentForHashGenerationException
+        */
+       public function validateAndStripHmacThrowsExceptionIfGivenStringIsTooShort() {
+               $this->hashService->validateAndStripHmac('string with less than 40 characters');
+       }
+
+       /**
+        * @test
+        * @expectedException TYPO3\FLOW3\Security\Exception\InvalidHashException
+        */
+       public function validateAndStripHmacThrowsExceptionIfGivenStringHasNoHashAppended() {
+               $this->hashService->validateAndStripHmac('string with exactly a length 40 of chars');
+       }
+
+       /**
+        * @test
+        * @expectedException TYPO3\FLOW3\Security\Exception\InvalidHashException
+        */
+       public function validateAndStripHmacThrowsExceptionIfTheAppendedHashIsInvalid() {
+               $this->hashService->validateAndStripHmac('some Stringac43682075d36592d4cb320e69ff0aa515886eab');
+       }
+
+       /**
+        * @test
+        */
+       public function validateAndStripHmacReturnsTheStringWithoutHmac() {
+               $string = ' Some arbitrary string with special characters: öäüß!"§$ ';
+               $hashedString = $this->hashService->appendHmac($string);
+               $actualResult = $this->hashService->validateAndStripHmac($hashedString);
+               $this->assertSame($string, $actualResult);
+       }
 }
 ?>
\ No newline at end of file