From: Christian Flothmann <christian.flothmann@sensiolabs.de>
Date: Fri, 1 Sep 2017 11:39:51 +0200
Subject: ensure that submitted data are uploaded files

[CVE-2017-16790] https://symfony.com/blog/cve-2017-16790-ensure-that-submitted-data-are-uploaded-files

Origin: backport, https://github.com/symfony/symfony/commit/f9e210cc7bbaa52adc2a6f695cc343854fa43df6
---
 UPGRADE-2.7.md                                     |   2 +
 src/Symfony/Component/Form/CHANGELOG.md            |   5 +
 .../Form/Extension/Core/Type/FileType.php          |  31 +++--
 .../HttpFoundationRequestHandler.php               |   9 +-
 .../Component/Form/NativeRequestHandler.php        |  21 ++--
 .../Component/Form/RequestHandlerInterface.php     |   7 ++
 .../Form/Tests/AbstractRequestHandlerTest.php      |  12 ++
 .../Tests/Extension/Core/Type/FileTypeTest.php     | 132 ++++++++++++++-------
 .../HttpFoundationRequestHandlerTest.php           |   5 +
 .../Form/Tests/NativeRequestHandlerTest.php        |  11 ++
 10 files changed, 175 insertions(+), 60 deletions(-)

diff --git a/UPGRADE-2.7.md b/UPGRADE-2.7.md
index 5de67eb..432c036 100644
--- a/UPGRADE-2.7.md
+++ b/UPGRADE-2.7.md
@@ -39,6 +39,8 @@ Router
 Form
 ----
 
+ * the `isFileUpload()` method was added to the `RequestHandlerInterface`
+
  * In form types and extension overriding the "setDefaultOptions" of the
    AbstractType or AbstractExtensionType has been deprecated in favor of
    overriding the new "configureOptions" method.
diff --git a/src/Symfony/Component/Form/CHANGELOG.md b/src/Symfony/Component/Form/CHANGELOG.md
index e79eec4..31e991d 100644
--- a/src/Symfony/Component/Form/CHANGELOG.md
+++ b/src/Symfony/Component/Form/CHANGELOG.md
@@ -12,6 +12,11 @@ CHANGELOG
  * moved data trimming logic of TrimListener into StringUtil
  * [BC BREAK] When registering a type extension through the DI extension, the tag alias has to match the actual extended type.
 
+2.7.38
+------
+
+ * [BC BREAK] the `isFileUpload()` method was added to the `RequestHandlerInterface`
+
 2.7.0
 -----
 
diff --git a/src/Symfony/Component/Form/Extension/Core/Type/FileType.php b/src/Symfony/Component/Form/Extension/Core/Type/FileType.php
index 40da512..b761f14 100644
--- a/src/Symfony/Component/Form/Extension/Core/Type/FileType.php
+++ b/src/Symfony/Component/Form/Extension/Core/Type/FileType.php
@@ -27,20 +27,35 @@ class FileType extends AbstractType
      */
     public function buildForm(FormBuilderInterface $builder, array $options)
     {
-        if ($options['multiple']) {
-            $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) {
-                $form = $event->getForm();
-                $data = $event->getData();
+        $builder->addEventListener(FormEvents::PRE_SUBMIT, function (FormEvent $event) use ($options) {
+            $form = $event->getForm();
+            $requestHandler = $form->getConfig()->getRequestHandler();
+            $data = null;
+
+            if ($options['multiple']) {
+                $data = array();
+
+                foreach ($event->getData() as $file) {
+                    if ($requestHandler->isFileUpload($file)) {
+                        $data[] = $file;
+                    }
+                }
 
                 // submitted data for an input file (not required) without choosing any file
-                if (array(null) === $data) {
+                if (array(null) === $data || array() === $data) {
                     $emptyData = $form->getConfig()->getEmptyData();
 
                     $data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
-                    $event->setData($data);
                 }
-            });
-        }
+
+                $event->setData($data);
+            } elseif (!$requestHandler->isFileUpload($event->getData())) {
+                $emptyData = $form->getConfig()->getEmptyData();
+
+                $data = is_callable($emptyData) ? call_user_func($emptyData, $form, $data) : $emptyData;
+                $event->setData($data);
+            }
+        });
     }
 
     /**
diff --git a/src/Symfony/Component/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php b/src/Symfony/Component/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php
index 98bbd4b..d30adaf 100644
--- a/src/Symfony/Component/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php
+++ b/src/Symfony/Component/Form/Extension/HttpFoundation/HttpFoundationRequestHandler.php
@@ -16,6 +16,7 @@ use Symfony\Component\Form\FormError;
 use Symfony\Component\Form\FormInterface;
 use Symfony\Component\Form\RequestHandlerInterface;
 use Symfony\Component\Form\Util\ServerParams;
+use Symfony\Component\HttpFoundation\File\File;
 use Symfony\Component\HttpFoundation\Request;
 
 /**
@@ -31,9 +32,6 @@ class HttpFoundationRequestHandler implements RequestHandlerInterface
      */
     private $serverParams;
 
-    /**
-     * {@inheritdoc}
-     */
     public function __construct(ServerParams $serverParams = null)
     {
         $this->serverParams = $serverParams ?: new ServerParams();
@@ -115,4 +113,9 @@ class HttpFoundationRequestHandler implements RequestHandlerInterface
 
         $form->submit($data, 'PATCH' !== $method);
     }
+
+    public function isFileUpload($data)
+    {
+        return $data instanceof File;
+    }
 }
diff --git a/src/Symfony/Component/Form/NativeRequestHandler.php b/src/Symfony/Component/Form/NativeRequestHandler.php
index c9a7685..dfff480 100644
--- a/src/Symfony/Component/Form/NativeRequestHandler.php
+++ b/src/Symfony/Component/Form/NativeRequestHandler.php
@@ -26,14 +26,6 @@ class NativeRequestHandler implements RequestHandlerInterface
      */
     private $serverParams;
 
-    /**
-     * {@inheritdoc}
-     */
-    public function __construct(ServerParams $params = null)
-    {
-        $this->serverParams = $params ?: new ServerParams();
-    }
-
     /**
      * The allowed keys of the $_FILES array.
      *
@@ -47,6 +39,11 @@ class NativeRequestHandler implements RequestHandlerInterface
         'type',
     );
 
+    public function __construct(ServerParams $params = null)
+    {
+        $this->serverParams = $params ?: new ServerParams();
+    }
+
     /**
      * {@inheritdoc}
      */
@@ -129,6 +126,14 @@ class NativeRequestHandler implements RequestHandlerInterface
         $form->submit($data, 'PATCH' !== $method);
     }
 
+    public function isFileUpload($data)
+    {
+        // POST data will always be strings or arrays of strings. Thus, we can be sure
+        // that the submitted data is a file upload if the "error" value is an integer
+        // (this value must have been injected by PHP itself).
+        return is_array($data) && isset($data['error']) && is_int($data['error']);
+    }
+
     /**
      * Returns the method used to submit the request to the server.
      *
diff --git a/src/Symfony/Component/Form/RequestHandlerInterface.php b/src/Symfony/Component/Form/RequestHandlerInterface.php
index d0a58e6..94c7bf3 100644
--- a/src/Symfony/Component/Form/RequestHandlerInterface.php
+++ b/src/Symfony/Component/Form/RequestHandlerInterface.php
@@ -25,4 +25,11 @@ interface RequestHandlerInterface
      * @param mixed         $request The current request.
      */
     public function handleRequest(FormInterface $form, $request = null);
+
+    /**
+     * @param mixed $data
+     *
+     * @return bool
+     */
+    public function isFileUpload($data);
 }
diff --git a/src/Symfony/Component/Form/Tests/AbstractRequestHandlerTest.php b/src/Symfony/Component/Form/Tests/AbstractRequestHandlerTest.php
index 022b514..0a4bcf2 100644
--- a/src/Symfony/Component/Form/Tests/AbstractRequestHandlerTest.php
+++ b/src/Symfony/Component/Form/Tests/AbstractRequestHandlerTest.php
@@ -355,12 +355,24 @@ abstract class AbstractRequestHandlerTest extends \PHPUnit_Framework_TestCase
         );
     }
 
+    public function testUploadedFilesAreAccepted()
+    {
+        $this->assertTrue($this->requestHandler->isFileUpload($this->getMockFile()));
+    }
+
+    public function testInvalidFilesAreRejected()
+    {
+        $this->assertFalse($this->requestHandler->isFileUpload($this->getInvalidFile()));
+    }
+
     abstract protected function setRequestData($method, $data, $files = array());
 
     abstract protected function getRequestHandler();
 
     abstract protected function getMockFile($suffix = '');
 
+    abstract protected function getInvalidFile();
+
     protected function getMockForm($name, $method = null, $compound = true)
     {
         $config = $this->getMock('Symfony\Component\Form\FormConfigInterface');
diff --git a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php
index cbeeb46..5156806 100644
--- a/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php
+++ b/src/Symfony/Component/Form/Tests/Extension/Core/Type/FileTypeTest.php
@@ -11,8 +11,15 @@
 
 namespace Symfony\Component\Form\Tests\Extension\Core\Type;
 
+use Symfony\Component\Form\Extension\HttpFoundation\HttpFoundationRequestHandler;
+use Symfony\Component\Form\NativeRequestHandler;
+use Symfony\Component\Form\RequestHandlerInterface;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+
 class FileTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
 {
+    const TESTED_TYPE = 'Symfony\Component\Form\Extension\Core\Type\FileType';
+
     /**
      * @group legacy
      */
@@ -26,18 +33,23 @@ class FileTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
     // https://github.com/symfony/symfony/pull/5028
     public function testSetData()
     {
-        $form = $this->factory->createBuilder('Symfony\Component\Form\Extension\Core\Type\FileType')->getForm();
-        $data = $this->createUploadedFileMock('abcdef', 'original.jpg', true);
+        $form = $this->factory->createBuilder(static::TESTED_TYPE)->getForm();
+        $data = $this->getMockBuilder('Symfony\Component\HttpFoundation\File\File')
+            ->setConstructorArgs(array(__DIR__.'/../../../Fixtures/foo', 'foo'))
+            ->getMock();
 
         $form->setData($data);
 
         $this->assertSame($data, $form->getData());
     }
 
-    public function testSubmit()
+    /**
+     * @dataProvider requestHandlerProvider
+     */
+    public function testSubmit(RequestHandlerInterface $requestHandler)
     {
-        $form = $this->factory->createBuilder('Symfony\Component\Form\Extension\Core\Type\FileType')->getForm();
-        $data = $this->createUploadedFileMock('abcdef', 'original.jpg', true);
+        $form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
+        $data = $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg');
 
         $form->submit($data);
 
@@ -66,30 +78,36 @@ class FileTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $this->assertSame(array(), $form->getData());
     }
 
-    public function testSetDataMultiple()
+    /**
+     * @dataProvider requestHandlerProvider
+     */
+    public function testSetDataMultiple(RequestHandlerInterface $requestHandler)
     {
         $form = $this->factory->createBuilder('file', null, array(
             'multiple' => true,
-        ))->getForm();
+        ))->setRequestHandler($requestHandler)->getForm();
 
         $data = array(
-            $this->createUploadedFileMock('abcdef', 'first.jpg', true),
-            $this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
+            $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
+            $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
         );
 
         $form->setData($data);
         $this->assertSame($data, $form->getData());
     }
 
-    public function testSubmitMultiple()
+    /**
+     * @dataProvider requestHandlerProvider
+     */
+    public function testSubmitMultiple(RequestHandlerInterface $requestHandler)
     {
         $form = $this->factory->createBuilder('Symfony\Component\Form\Extension\Core\Type\FileType', null, array(
             'multiple' => true,
-        ))->getForm();
+        ))->setRequestHandler($requestHandler)->getForm();
 
         $data = array(
-            $this->createUploadedFileMock('abcdef', 'first.jpg', true),
-            $this->createUploadedFileMock('zyxwvu', 'second.jpg', true),
+            $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
+            $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
         );
 
         $form->submit($data);
@@ -100,40 +118,72 @@ class FileTypeTest extends \Symfony\Component\Form\Test\TypeTestCase
         $this->assertArrayHasKey('multiple', $view->vars['attr']);
     }
 
-    public function testDontPassValueToView()
+    /**
+     * @dataProvider requestHandlerProvider
+     */
+    public function testDontPassValueToView(RequestHandlerInterface $requestHandler)
     {
-        $form = $this->factory->create('Symfony\Component\Form\Extension\Core\Type\FileType');
+        $form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
         $form->submit(array(
-            'Symfony\Component\Form\Extension\Core\Type\FileType' => $this->createUploadedFileMock('abcdef', 'original.jpg', true),
+            'file' => $this->createUploadedFileMock($requestHandler, __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
         ));
-        $view = $form->createView();
 
-        $this->assertEquals('', $view->vars['value']);
+        $this->assertEquals('', $form->createView()->vars['value']);
     }
 
-    private function createUploadedFileMock($name, $originalName, $valid)
+    /**
+     * @dataProvider requestHandlerProvider
+     */
+    public function testSubmittedFilePathsAreDropped(RequestHandlerInterface $requestHandler)
     {
-        $file = $this
-            ->getMockBuilder('Symfony\Component\HttpFoundation\File\UploadedFile')
-            ->setConstructorArgs(array(__DIR__.'/../../../Fixtures/foo', 'foo'))
-            ->getMock()
-        ;
-        $file
-            ->expects($this->any())
-            ->method('getBasename')
-            ->will($this->returnValue($name))
-        ;
-        $file
-            ->expects($this->any())
-            ->method('getClientOriginalName')
-            ->will($this->returnValue($originalName))
-        ;
-        $file
-            ->expects($this->any())
-            ->method('isValid')
-            ->will($this->returnValue($valid))
-        ;
-
-        return $file;
+        $form = $this->factory->createBuilder(static::TESTED_TYPE)->setRequestHandler($requestHandler)->getForm();
+        $form->submit('file:///etc/passwd');
+
+        $this->assertNull($form->getData());
+        $this->assertNull($form->getNormData());
+        $this->assertSame('', $form->getViewData());
+    }
+
+    /**
+     * @dataProvider requestHandlerProvider
+     */
+    public function testMultipleSubmittedFilePathsAreDropped(RequestHandlerInterface $requestHandler)
+    {
+        $form = $this->factory
+            ->createBuilder(static::TESTED_TYPE, null, array(
+                'multiple' => true,
+            ))
+            ->setRequestHandler($requestHandler)
+            ->getForm();
+        $form->submit(array(
+            'file:///etc/passwd',
+            $this->createUploadedFileMock(new HttpFoundationRequestHandler(), __DIR__.'/../../../Fixtures/foo', 'foo.jpg'),
+            $this->createUploadedFileMock(new NativeRequestHandler(), __DIR__.'/../../../Fixtures/foo2', 'foo2.jpg'),
+        ));
+
+        $this->assertCount(1, $form->getData());
+    }
+
+    public function requestHandlerProvider()
+    {
+        return array(
+            array(new HttpFoundationRequestHandler()),
+            array(new NativeRequestHandler()),
+        );
+    }
+
+    private function createUploadedFileMock(RequestHandlerInterface $requestHandler, $path, $originalName)
+    {
+        if ($requestHandler instanceof HttpFoundationRequestHandler) {
+            return new UploadedFile($path, $originalName, null, 10, null, true);
+        }
+
+        return array(
+            'name' => $originalName,
+            'error' => 0,
+            'type' => 'text/plain',
+            'tmp_name' => $path,
+            'size' => 10,
+        );
     }
 }
diff --git a/src/Symfony/Component/Form/Tests/Extension/HttpFoundation/HttpFoundationRequestHandlerTest.php b/src/Symfony/Component/Form/Tests/Extension/HttpFoundation/HttpFoundationRequestHandlerTest.php
index e71a1fd..9fbe122 100644
--- a/src/Symfony/Component/Form/Tests/Extension/HttpFoundation/HttpFoundationRequestHandlerTest.php
+++ b/src/Symfony/Component/Form/Tests/Extension/HttpFoundation/HttpFoundationRequestHandlerTest.php
@@ -51,4 +51,9 @@ class HttpFoundationRequestHandlerTest extends AbstractRequestHandlerTest
     {
         return new UploadedFile(__DIR__.'/../../Fixtures/foo'.$suffix, 'foo'.$suffix);
     }
+
+    protected function getInvalidFile()
+    {
+        return 'file:///etc/passwd';
+    }
 }
diff --git a/src/Symfony/Component/Form/Tests/NativeRequestHandlerTest.php b/src/Symfony/Component/Form/Tests/NativeRequestHandlerTest.php
index 3858006..aba4172 100644
--- a/src/Symfony/Component/Form/Tests/NativeRequestHandlerTest.php
+++ b/src/Symfony/Component/Form/Tests/NativeRequestHandlerTest.php
@@ -216,4 +216,15 @@ class NativeRequestHandlerTest extends AbstractRequestHandlerTest
             'size' => 100,
         );
     }
+
+    protected function getInvalidFile()
+    {
+        return array(
+            'name' => 'upload.txt',
+            'type' => 'text/plain',
+            'tmp_name' => 'owfdskjasdfsa',
+            'error' => '0',
+            'size' => '100',
+        );
+    }
 }