Skip to content

Commit

Permalink
bug #19373 [Form] Skip CSRF validation on form when POST max size is …
Browse files Browse the repository at this point in the history
…exceeded (jameshalsall)

This PR was squashed before being merged into the 2.7 branch (closes #19373).

Discussion
----------

[Form] Skip CSRF validation on form when POST max size is exceeded

| Q             | A
| ------------- | ---
| Branch?       | 2.7
| Bug fix?      | yes
| New feature?  | no
| BC breaks?    | no
| Deprecations? | no
| Tests pass?   | yes
| Fixed tickets | #19140
| License       | MIT
| Doc PR        | N/A

In #19140 the CSRF validation listener was not aware that the POST max size had exceeded, and was adding a form error message that wasn't relevant to the actual error.

This introduces the `ServerParams` utility class into the `CsrfValidationListener` and checks that the POST max size has not been exceeded. If it has then it won't bother trying to validate the CSRF token.

My main concern with this change is that it opens up an attack vector around tokens, but I've encapsulated the request size validation in a single method in `ServerParams` now so that the request handlers are using the same logic.

Commits
-------

289531f [Form] Skip CSRF validation on form when POST max size is exceeded
  • Loading branch information
fabpot committed Aug 15, 2016
2 parents b405df0 + 289531f commit 1a059e5
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 12 deletions.
Expand Up @@ -16,6 +16,7 @@
<argument>%form.type_extension.csrf.field_name%</argument>
<argument type="service" id="translator.default" />
<argument>%validator.translation_domain%</argument>
<argument type="service" id="form.server_params" />
</service>
</services>
</container>
Expand Up @@ -18,6 +18,7 @@
use Symfony\Component\Form\FormEvents;
use Symfony\Component\Form\FormError;
use Symfony\Component\Form\FormEvent;
use Symfony\Component\Form\Util\ServerParams;
use Symfony\Component\Security\Csrf\CsrfToken;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
use Symfony\Component\Translation\TranslatorInterface;
Expand Down Expand Up @@ -68,14 +69,19 @@ class CsrfValidationListener implements EventSubscriberInterface
*/
private $translationDomain;

/**
* @var ServerParams
*/
private $serverParams;

public static function getSubscribedEvents()
{
return array(
FormEvents::PRE_SUBMIT => 'preSubmit',
);
}

public function __construct($fieldName, $tokenManager, $tokenId, $errorMessage, TranslatorInterface $translator = null, $translationDomain = null)
public function __construct($fieldName, $tokenManager, $tokenId, $errorMessage, TranslatorInterface $translator = null, $translationDomain = null, ServerParams $serverParams = null)
{
if ($tokenManager instanceof CsrfProviderInterface) {
$tokenManager = new CsrfProviderAdapter($tokenManager);
Expand All @@ -89,13 +95,15 @@ public function __construct($fieldName, $tokenManager, $tokenId, $errorMessage,
$this->errorMessage = $errorMessage;
$this->translator = $translator;
$this->translationDomain = $translationDomain;
$this->serverParams = $serverParams ?: new ServerParams();
}

public function preSubmit(FormEvent $event)
{
$form = $event->getForm();
$postRequestSizeExceeded = $form->getConfig()->getMethod() === 'POST' && $this->serverParams->hasPostMaxSizeBeenExceeded();

if ($form->isRoot() && $form->getConfig()->getOption('compound')) {
if ($form->isRoot() && $form->getConfig()->getOption('compound') && !$postRequestSizeExceeded) {
$data = $event->getData();

if (!isset($data[$this->fieldName]) || !$this->tokenManager->isTokenValid(new CsrfToken($this->tokenId, $data[$this->fieldName]))) {
Expand Down
Expand Up @@ -20,6 +20,7 @@
use Symfony\Component\Form\FormBuilderInterface;
use Symfony\Component\Form\FormView;
use Symfony\Component\Form\FormInterface;
use Symfony\Component\Form\Util\ServerParams;
use Symfony\Component\OptionsResolver\Options;
use Symfony\Component\OptionsResolver\OptionsResolver;
use Symfony\Component\Security\Csrf\CsrfTokenManagerInterface;
Expand Down Expand Up @@ -55,7 +56,12 @@ class FormTypeCsrfExtension extends AbstractTypeExtension
*/
private $translationDomain;

public function __construct($defaultTokenManager, $defaultEnabled = true, $defaultFieldName = '_token', TranslatorInterface $translator = null, $translationDomain = null)
/**
* @var ServerParams
*/
private $serverParams;

public function __construct($defaultTokenManager, $defaultEnabled = true, $defaultFieldName = '_token', TranslatorInterface $translator = null, $translationDomain = null, ServerParams $serverParams = null)
{
if ($defaultTokenManager instanceof CsrfProviderInterface) {
$defaultTokenManager = new CsrfProviderAdapter($defaultTokenManager);
Expand All @@ -68,6 +74,7 @@ public function __construct($defaultTokenManager, $defaultEnabled = true, $defau
$this->defaultFieldName = $defaultFieldName;
$this->translator = $translator;
$this->translationDomain = $translationDomain;
$this->serverParams = $serverParams;
}

/**
Expand All @@ -89,7 +96,8 @@ public function buildForm(FormBuilderInterface $builder, array $options)
$options['csrf_token_id'] ?: ($builder->getName() ?: get_class($builder->getType()->getInnerType())),
$options['csrf_message'],
$this->translator,
$this->translationDomain
$this->translationDomain,
$this->serverParams
))
;
}
Expand Down
Expand Up @@ -73,10 +73,7 @@ public function handleRequest(FormInterface $form, $request = null)
// Mark the form with an error if the uploaded size was too large
// This is done here and not in FormValidator because $_POST is
// empty when that error occurs. Hence the form is never submitted.
$contentLength = $this->serverParams->getContentLength();
$maxContentLength = $this->serverParams->getPostMaxSize();

if (!empty($maxContentLength) && $contentLength > $maxContentLength) {
if ($this->serverParams->hasPostMaxSizeBeenExceeded()) {
// Submit the form, but don't clear the default values
$form->submit(null, false);

Expand Down
5 changes: 1 addition & 4 deletions src/Symfony/Component/Form/NativeRequestHandler.php
Expand Up @@ -81,10 +81,7 @@ public function handleRequest(FormInterface $form, $request = null)
// Mark the form with an error if the uploaded size was too large
// This is done here and not in FormValidator because $_POST is
// empty when that error occurs. Hence the form is never submitted.
$contentLength = $this->serverParams->getContentLength();
$maxContentLength = $this->serverParams->getPostMaxSize();

if (!empty($maxContentLength) && $contentLength > $maxContentLength) {
if ($this->serverParams->hasPostMaxSizeBeenExceeded()) {
// Submit the form, but don't clear the default values
$form->submit(null, false);

Expand Down
Expand Up @@ -11,6 +11,7 @@

namespace Symfony\Component\Form\Tests\Extension\Csrf\EventListener;

use Symfony\Component\Form\Form;
use Symfony\Component\Form\FormBuilder;
use Symfony\Component\Form\FormEvent;
use Symfony\Component\Form\Extension\Csrf\EventListener\CsrfValidationListener;
Expand Down Expand Up @@ -72,4 +73,25 @@ public function testStringFormData()
// Validate accordingly
$this->assertSame($data, $event->getData());
}

public function testMaxPostSizeExceeded()
{
$serverParams = $this
->getMockBuilder('\Symfony\Component\Form\Util\ServerParams')
->disableOriginalConstructor()
->getMock()
;

$serverParams
->expects($this->once())
->method('hasPostMaxSizeBeenExceeded')
->willReturn(true)
;

$event = new FormEvent($this->form, array('csrf' => 'token'));
$validation = new CsrfValidationListener('csrf', $this->tokenManager, 'unknown', 'Error message', null, null, $serverParams);

$validation->preSubmit($event);
$this->assertEmpty($this->form->getErrors());
}
}
13 changes: 13 additions & 0 deletions src/Symfony/Component/Form/Util/ServerParams.php
Expand Up @@ -25,6 +25,19 @@ public function __construct(RequestStack $requestStack = null)
$this->requestStack = $requestStack;
}

/**
* Returns true if the POST max size has been exceeded in the request.
*
* @return bool
*/
public function hasPostMaxSizeBeenExceeded()
{
$contentLength = $this->getContentLength();
$maxContentLength = $this->getPostMaxSize();

return $maxContentLength && $contentLength > $maxContentLength;
}

/**
* Returns maximum post size in bytes.
*
Expand Down

0 comments on commit 1a059e5

Please sign in to comment.