From f615301fd5ec86f5c2070c76d128bb266c8d72de Mon Sep 17 00:00:00 2001
From: Helmut Hummel <info@helhum.io>
Date: Tue, 24 May 2016 09:44:23 +0200
Subject: [PATCH] [SECURITY] Validate complete referring request
Instead of only checking for valid request arguments by using a hmac,
we now check the complete request including action, controller and vendor
to avoid spoofing these arguments and bypassing other security checks
during forwarding to the referring action.
Additionally, ReferringRequest is now separate from regular Request.
The meaning of properties starting with "@" is only valid for
processing a referring request. To avoid mixed concerns in using
the same Request implementation for regular requests and referring
requests, they are separated now.
Resolves: #76231
Resolves: #76256
Releases: master, 7.6, 6.2
Security-Commit: 3562e177f1720e62cab84232dcc67c580a3cc3db
Security-Bulletin: TYPO3-CORE-SA-2016-013
Change-Id: Ic94e11341df98c1326dc73c92a5c9e061a64cc9e
Reviewed-on: https://review.typo3.org/48258
Reviewed-by: Oliver Hader <oliver.hader@typo3.org>
Tested-by: Oliver Hader <oliver.hader@typo3.org>
---
typo3/sysext/extbase/Classes/Mvc/Request.php | 23 +-------
.../Classes/Mvc/Web/ReferringRequest.php | 56 +++++++++++++++++++
.../extbase/Classes/Mvc/Web/Request.php | 16 +++---
.../Classes/ViewHelpers/FormViewHelper.php | 37 ++++++++++++
.../Unit/ViewHelpers/FormViewHelperTest.php | 14 +++--
5 files changed, 111 insertions(+), 35 deletions(-)
create mode 100644 typo3/sysext/extbase/Classes/Mvc/Web/ReferringRequest.php
diff --git a/typo3/sysext/extbase/Classes/Mvc/Request.php b/typo3/sysext/extbase/Classes/Mvc/Request.php
index ebddbfedfea5..1b06e3b8e6ec 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Request.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Request.php
@@ -390,27 +390,8 @@ class Request implements RequestInterface
$this->internalArguments[$argumentName] = $value;
return;
}
- switch ($argumentName) {
- case '@extension':
- $this->setControllerExtensionName($value);
- break;
- case '@subpackage':
- $this->setControllerSubpackageKey($value);
- break;
- case '@controller':
- $this->setControllerName($value);
- break;
- case '@action':
- $this->setControllerActionName($value);
- break;
- case '@format':
- $this->setFormat($value);
- break;
- case '@vendor':
- $this->setControllerVendorName($value);
- break;
- default:
- $this->arguments[$argumentName] = $value;
+ if ($argumentName[0] !== '@') {
+ $this->arguments[$argumentName] = $value;
}
}
diff --git a/typo3/sysext/extbase/Classes/Mvc/Web/ReferringRequest.php b/typo3/sysext/extbase/Classes/Mvc/Web/ReferringRequest.php
new file mode 100644
index 000000000000..085b67dce4ff
--- /dev/null
+++ b/typo3/sysext/extbase/Classes/Mvc/Web/ReferringRequest.php
@@ -0,0 +1,56 @@
+<?php
+namespace TYPO3\CMS\Extbase\Mvc\Web;
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+/**
+ * Represents a referring web request.
+ *
+ * @api
+ */
+class ReferringRequest extends Request
+{
+ /**
+ * Sets the value of the specified argument
+ *
+ * @param string $argumentName Name of the argument to set
+ * @param mixed $value The new value
+ */
+ public function setArgument($argumentName, $value)
+ {
+ parent::setArgument($argumentName, $value);
+
+ switch ($argumentName) {
+ case '@extension':
+ $this->setControllerExtensionName($value);
+ break;
+ case '@subpackage':
+ $this->setControllerSubpackageKey($value);
+ break;
+ case '@controller':
+ $this->setControllerName($value);
+ break;
+ case '@action':
+ $this->setControllerActionName($value);
+ break;
+ case '@format':
+ $this->setFormat($value);
+ break;
+ case '@vendor':
+ $this->setControllerVendorName($value);
+ break;
+ }
+ }
+
+}
diff --git a/typo3/sysext/extbase/Classes/Mvc/Web/Request.php b/typo3/sysext/extbase/Classes/Mvc/Web/Request.php
index bafa350eb20c..0481910c015f 100644
--- a/typo3/sysext/extbase/Classes/Mvc/Web/Request.php
+++ b/typo3/sysext/extbase/Classes/Mvc/Web/Request.php
@@ -183,20 +183,18 @@ class Request extends \TYPO3\CMS\Extbase\Mvc\Request
/**
* Get a freshly built request object pointing to the Referrer.
*
- * @return Request the referring request, or NULL if no referrer found
+ * @return ReferringRequest the referring request, or null if no referrer found
*/
public function getReferringRequest()
{
- if (isset($this->internalArguments['__referrer']) && is_array($this->internalArguments['__referrer'])) {
- $referrerArray = $this->internalArguments['__referrer'];
- $referringRequest = new \TYPO3\CMS\Extbase\Mvc\Web\Request();
+ if (isset($this->internalArguments['__referrer']['@request'])) {
+ $referrerArray = unserialize($this->hashService->validateAndStripHmac($this->internalArguments['__referrer']['@request']));
$arguments = array();
- if (isset($referrerArray['arguments'])) {
- $serializedArgumentsWithHmac = $referrerArray['arguments'];
- $serializedArguments = $this->hashService->validateAndStripHmac($serializedArgumentsWithHmac);
- $arguments = unserialize(base64_decode($serializedArguments));
- unset($referrerArray['arguments']);
+ if (isset($this->internalArguments['__referrer']['arguments'])) {
+ // This case is kept for compatibility in 7.6 and 6.2, but will be removed in 8
+ $arguments = unserialize(base64_decode($this->hashService->validateAndStripHmac($this->internalArguments['__referrer']['arguments'])));
}
+ $referringRequest = new ReferringRequest();
$referringRequest->setArguments(\TYPO3\CMS\Extbase\Utility\ArrayUtility::arrayMergeRecursiveOverrule($arguments, $referrerArray));
return $referringRequest;
}
diff --git a/typo3/sysext/fluid/Classes/ViewHelpers/FormViewHelper.php b/typo3/sysext/fluid/Classes/ViewHelpers/FormViewHelper.php
index a0ad3e41aa80..ea757f52708e 100644
--- a/typo3/sysext/fluid/Classes/ViewHelpers/FormViewHelper.php
+++ b/typo3/sysext/fluid/Classes/ViewHelpers/FormViewHelper.php
@@ -71,6 +71,11 @@ class FormViewHelper extends \TYPO3\CMS\Fluid\ViewHelpers\Form\AbstractFormViewH
*/
protected $formActionUriArguments;
+ /**
+ * @var bool
+ */
+ private $securedReferrerFieldRendered = false;
+
/**
* @param \TYPO3\CMS\Extbase\Security\Cryptography\HashService $hashService
*/
@@ -159,6 +164,7 @@ class FormViewHelper extends \TYPO3\CMS\Fluid\ViewHelpers\Form\AbstractFormViewH
$content .= $this->renderHiddenIdentityField($this->arguments['object'], $this->getFormObjectName());
$content .= $this->renderAdditionalIdentityFields();
$content .= $this->renderHiddenReferrerFields();
+ $content .= $this->renderHiddenSecuredReferrerField();
// Render the trusted list of all properties after everything else has been rendered
$content .= $this->renderTrustedPropertiesField();
@@ -232,7 +238,38 @@ class FormViewHelper extends \TYPO3\CMS\Fluid\ViewHelpers\Form\AbstractFormViewH
$result .= '<input type="hidden" name="' . $this->prefixFieldName('__referrer[@controller]') . '" value="' . $controllerName . '" />' . LF;
$result .= '<input type="hidden" name="' . $this->prefixFieldName('__referrer[@action]') . '" value="' . $actionName . '" />' . LF;
$result .= '<input type="hidden" name="' . $this->prefixFieldName('__referrer[arguments]') . '" value="' . htmlspecialchars($this->hashService->appendHmac(base64_encode(serialize($request->getArguments())))) . '" />' . LF;
+ $result .= $this->renderHiddenSecuredReferrerField();
+
+ return $result;
+ }
+ /**
+ * Renders hidden form field for secured referrer information about the current controller and action.
+ *
+ * This method is called twice, to deal with subclasses of this class in a most compatible way
+ *
+ * @return string Hidden field with secured referrer information
+ */
+ protected function renderHiddenSecuredReferrerField()
+ {
+ if ($this->securedReferrerFieldRendered) {
+ return '';
+ }
+ $request = $this->renderingContext->getControllerContext()->getRequest();
+ $extensionName = $request->getControllerExtensionName();
+ $vendorName = $request->getControllerVendorName();
+ $controllerName = $request->getControllerName();
+ $actionName = $request->getControllerActionName();
+ $actionRequest = [
+ '@extension' => $extensionName,
+ '@controller' => $controllerName,
+ '@action' => $actionName,
+ ];
+ if ($vendorName !== null) {
+ $actionRequest['@vendor'] = $vendorName;
+ }
+ $result = '<input type="hidden" name="' . $this->prefixFieldName('__referrer[@request]') . '" value="' . htmlspecialchars($this->hashService->appendHmac(serialize($actionRequest))) . '" />' . LF;
+ $this->securedReferrerFieldRendered = true;
return $result;
}
diff --git a/typo3/sysext/fluid/Tests/Unit/ViewHelpers/FormViewHelperTest.php b/typo3/sysext/fluid/Tests/Unit/ViewHelpers/FormViewHelperTest.php
index cd7d0019c357..d6ac7c26835e 100644
--- a/typo3/sysext/fluid/Tests/Unit/ViewHelpers/FormViewHelperTest.php
+++ b/typo3/sysext/fluid/Tests/Unit/ViewHelpers/FormViewHelperTest.php
@@ -88,7 +88,7 @@ class FormViewHelperTest extends ViewHelperBaseTestcase
public function renderAddsObjectToViewHelperVariableContainer()
{
$formObject = new \stdClass();
- $viewHelper = $this->getAccessibleMock(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class, array('renderChildren', 'renderHiddenIdentityField', 'renderAdditionalIdentityFields', 'renderHiddenReferrerFields', 'renderRequestHashField', 'addFormObjectNameToViewHelperVariableContainer', 'addFieldNamePrefixToViewHelperVariableContainer', 'removeFormObjectNameFromViewHelperVariableContainer', 'removeFieldNamePrefixFromViewHelperVariableContainer', 'addFormFieldNamesToViewHelperVariableContainer', 'removeFormFieldNamesFromViewHelperVariableContainer', 'renderTrustedPropertiesField'), array(), '', false);
+ $viewHelper = $this->getAccessibleMock(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class, array('renderChildren', 'renderHiddenIdentityField', 'renderAdditionalIdentityFields', 'renderHiddenReferrerFields', 'renderHiddenSecuredReferrerField', 'renderRequestHashField', 'addFormObjectNameToViewHelperVariableContainer', 'addFieldNamePrefixToViewHelperVariableContainer', 'removeFormObjectNameFromViewHelperVariableContainer', 'removeFieldNamePrefixFromViewHelperVariableContainer', 'addFormFieldNamesToViewHelperVariableContainer', 'removeFormFieldNamesFromViewHelperVariableContainer', 'renderTrustedPropertiesField'), array(), '', false);
$this->injectDependenciesIntoViewHelper($viewHelper);
$viewHelper->setArguments(array('object' => $formObject));
$this->viewHelperVariableContainer->expects($this->at(0))->method('add')->with(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class, 'formObject', $formObject);
@@ -104,7 +104,7 @@ class FormViewHelperTest extends ViewHelperBaseTestcase
public function renderAddsObjectNameToTemplateVariableContainer()
{
$objectName = 'someObjectName';
- $viewHelper = $this->getAccessibleMock(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class, array('renderChildren', 'renderHiddenIdentityField', 'renderHiddenReferrerFields', 'renderRequestHashField', 'addFormObjectToViewHelperVariableContainer', 'addFieldNamePrefixToViewHelperVariableContainer', 'removeFormObjectFromViewHelperVariableContainer', 'removeFieldNamePrefixFromViewHelperVariableContainer', 'addFormFieldNamesToViewHelperVariableContainer', 'removeFormFieldNamesFromViewHelperVariableContainer', 'renderTrustedPropertiesField'), array(), '', false);
+ $viewHelper = $this->getAccessibleMock(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class, array('renderChildren', 'renderHiddenIdentityField', 'renderHiddenReferrerFields', 'renderHiddenSecuredReferrerField', 'renderRequestHashField', 'addFormObjectToViewHelperVariableContainer', 'addFieldNamePrefixToViewHelperVariableContainer', 'removeFormObjectFromViewHelperVariableContainer', 'removeFieldNamePrefixFromViewHelperVariableContainer', 'addFormFieldNamesToViewHelperVariableContainer', 'removeFormFieldNamesFromViewHelperVariableContainer', 'renderTrustedPropertiesField'), array(), '', false);
$this->injectDependenciesIntoViewHelper($viewHelper);
$viewHelper->setArguments(array('name' => $objectName));
$this->viewHelperVariableContainer->expects($this->once())->method('add')->with(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class, 'formObjectName', $objectName);
@@ -167,7 +167,7 @@ class FormViewHelperTest extends ViewHelperBaseTestcase
*/
public function renderWrapsHiddenFieldsWithDivForXhtmlCompatibilityWithRewrittenPropertyMapper()
{
- $viewHelper = $this->getAccessibleMock($this->buildAccessibleProxy(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class), array('renderChildren', 'renderHiddenIdentityField', 'renderAdditionalIdentityFields', 'renderHiddenReferrerFields', 'renderTrustedPropertiesField'), array(), '', false);
+ $viewHelper = $this->getAccessibleMock($this->buildAccessibleProxy(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class), array('renderChildren', 'renderHiddenIdentityField', 'renderAdditionalIdentityFields', 'renderHiddenReferrerFields', 'renderHiddenSecuredReferrerField', 'renderTrustedPropertiesField'), array(), '', false);
$this->mvcPropertyMapperConfigurationService->_set('hashService', new \TYPO3\CMS\Extbase\Security\Cryptography\HashService());
$viewHelper->_set('mvcPropertyMapperConfigurationService', $this->mvcPropertyMapperConfigurationService);
parent::injectDependenciesIntoViewHelper($viewHelper);
@@ -185,7 +185,7 @@ class FormViewHelperTest extends ViewHelperBaseTestcase
*/
public function renderWrapsHiddenFieldsWithDivAndAnAdditionalClassForXhtmlCompatibilityWithRewrittenPropertyMapper()
{
- $viewHelper = $this->getMock($this->buildAccessibleProxy(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class), array('renderChildren', 'renderHiddenIdentityField', 'renderAdditionalIdentityFields', 'renderHiddenReferrerFields', 'renderTrustedPropertiesField'), array(), '', false);
+ $viewHelper = $this->getMock($this->buildAccessibleProxy(\TYPO3\CMS\Fluid\ViewHelpers\FormViewHelper::class), array('renderChildren', 'renderHiddenIdentityField', 'renderAdditionalIdentityFields', 'renderHiddenReferrerFields', 'renderHiddenSecuredReferrerField', 'renderTrustedPropertiesField'), array(), '', false);
$this->mvcPropertyMapperConfigurationService->_set('hashService', new \TYPO3\CMS\Extbase\Security\Cryptography\HashService());
$viewHelper->_set('mvcPropertyMapperConfigurationService', $this->mvcPropertyMapperConfigurationService);
parent::injectDependenciesIntoViewHelper($viewHelper);
@@ -228,7 +228,11 @@ class FormViewHelperTest extends ViewHelperBaseTestcase
$this->request->expects($this->atLeastOnce())->method('getControllerName')->will($this->returnValue('controllerName'));
$this->request->expects($this->atLeastOnce())->method('getControllerActionName')->will($this->returnValue('controllerActionName'));
$hiddenFields = $viewHelper->_call('renderHiddenReferrerFields');
- $expectedResult = chr(10) . '<input type="hidden" name="__referrer[@extension]" value="extensionName" />' . chr(10) . '<input type="hidden" name="__referrer[@controller]" value="controllerName" />' . chr(10) . '<input type="hidden" name="__referrer[@action]" value="controllerActionName" />' . chr(10) . '<input type="hidden" name="__referrer[arguments]" value="" />' . chr(10);
+ $expectedResult = chr(10) . '<input type="hidden" name="__referrer[@extension]" value="extensionName" />'
+ . chr(10) . '<input type="hidden" name="__referrer[@controller]" value="controllerName" />'
+ . chr(10) . '<input type="hidden" name="__referrer[@action]" value="controllerActionName" />'
+ . chr(10) . '<input type="hidden" name="__referrer[arguments]" value="" />'
+ . chr(10) . '<input type="hidden" name="__referrer[@request]" value="" />' . chr(10);
$this->assertEquals($expectedResult, $hiddenFields);
}
--
GitLab