Skip to content

Commit

Permalink
security #cve-2018-19789 [Form] Filter file uploads out of regular fo…
Browse files Browse the repository at this point in the history
…rm types (nicolas-grekas)

This PR was merged into the 2.7 branch.

Discussion
----------

[Form] Filter file uploads out of regular form types

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

This PR filters uploaded files out of the data processed by any form type except `FileType`.

Commits
-------

205a44e [Form] Filter file uploads out of regular form types
  • Loading branch information
nicolas-grekas committed Dec 6, 2018
2 parents cb8302c + 205a44e commit b65e6f1
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 1 deletion.
Expand Up @@ -105,6 +105,7 @@ public function configureOptions(OptionsResolver $resolver)
'data_class' => $dataClass,
'empty_data' => $emptyData,
'multiple' => false,
'allow_file_upload' => true,
));
}

Expand Down
Expand Up @@ -213,6 +213,7 @@ public function configureOptions(OptionsResolver $resolver)
'attr' => $defaultAttr,
'post_max_size_message' => 'The uploaded file was too large. Please try to upload a smaller file.',
'upload_max_size_message' => $uploadMaxSizeMessage, // internal
'allow_file_upload' => false,
));

$resolver->setAllowedTypes('label_attr', 'array');
Expand Down
9 changes: 9 additions & 0 deletions src/Symfony/Component/Form/Form.php
Expand Up @@ -541,6 +541,11 @@ public function submit($submittedData, $clearMissing = true)
$submittedData = null;
} elseif (is_scalar($submittedData)) {
$submittedData = (string) $submittedData;
} elseif ($this->config->getOption('allow_file_upload')) {
// no-op
} elseif ($this->config->getRequestHandler()->isFileUpload($submittedData)) {
$submittedData = null;
$this->transformationFailure = new TransformationFailedException('Submitted data was expected to be text or number, file upload given.');
}

$dispatcher = $this->config->getEventDispatcher();
Expand All @@ -550,6 +555,10 @@ public function submit($submittedData, $clearMissing = true)
$viewData = null;

try {
if (null !== $this->transformationFailure) {
throw $this->transformationFailure;
}

// Hook to change content of the data submitted by the browser
if ($dispatcher->hasListeners(FormEvents::PRE_SUBMIT)) {
$event = new FormEvent($this, $submittedData);
Expand Down
17 changes: 16 additions & 1 deletion src/Symfony/Component/Form/Tests/CompoundFormTest.php
Expand Up @@ -712,7 +712,7 @@ public function testSubmitPostOrPutRequestWithSingleChildForm($method)
'REQUEST_METHOD' => $method,
));

$form = $this->getBuilder('image')
$form = $this->getBuilder('image', null, null, array('allow_file_upload' => true))
->setMethod($method)
->setRequestHandler(new HttpFoundationRequestHandler())
->getForm();
Expand Down Expand Up @@ -1088,6 +1088,21 @@ public function testDisabledButtonIsNotSubmitted()
$this->assertFalse($submit->isSubmitted());
}

public function testFileUpload()
{
$reqHandler = new HttpFoundationRequestHandler();
$this->form->add($this->getBuilder('foo')->setRequestHandler($reqHandler)->getForm());
$this->form->add($this->getBuilder('bar')->setRequestHandler($reqHandler)->getForm());

$this->form->submit(array(
'foo' => 'Foo',
'bar' => new UploadedFile(__FILE__, 'upload.png', 'image/png', 123, UPLOAD_ERR_OK),
));

$this->assertSame('Submitted data was expected to be text or number, file upload given.', $this->form->get('bar')->getTransformationFailure()->getMessage());
$this->assertNull($this->form->get('bar')->getData());
}

protected function createForm()
{
return $this->getBuilder()
Expand Down

0 comments on commit b65e6f1

Please sign in to comment.