MAGETWO-95637: Validate file extension before upload

Author: viktorpetryk

app/code/Magento/CatalogImportExport/Model/Import/Uploader.php

@@ -17,6 +17,13 @@
  */
 class Uploader extends \Magento\MediaStorage\Model\File\Uploader
 {
+    /**
+     * HTTP scheme
+     * used to compare against the filename and select the proper DriverPool adapter
+     * @var string
+     */
+    private $httpScheme = 'http://';
+
     /**
      * Temp directory.
      *
@@ -99,7 +106,7 @@ class Uploader extends \Magento\MediaStorage\Model\File\Uploader
      * @param \Magento\MediaStorage\Model\File\Validator\NotProtectedExtension $validator
      * @param \Magento\Framework\Filesystem $filesystem
      * @param \Magento\Framework\Filesystem\File\ReadFactory $readFactory
-     * @param null $filePath
+     * @param string|null $filePath
      * @param \Magento\Framework\App\Filesystem\DirectoryResolver|null $directoryResolver
      * @throws \Magento\Framework\Exception\LocalizedException
      */
@@ -148,23 +155,38 @@ public function init()
      * @param string $fileName
      * @param bool $renameFileOff
      * @return array
+     * @throws \Magento\Framework\Exception\LocalizedException
      */
     public function move($fileName, $renameFileOff = false)
     {
         if ($renameFileOff) {
             $this->setAllowRenameFiles(false);
         }
+
+        if ($this->getTmpDir()) {
+            $filePath = $this->getTmpDir() . '/';
+        } else {
+            $filePath = '';
+ 		}
+
         if (preg_match('/\bhttps?:\/\//i', $fileName, $matches)) {
             $url = str_replace($matches[0], '', $fileName);
-            $read = $this->_readFactory->create($url, DriverPool::HTTP);
+            $driver = $matches[0] === $this->httpScheme ? DriverPool::HTTP : DriverPool::HTTPS;
+            $read = $this->_readFactory->create($url, $driver);
+
+            $fileExtension = pathinfo($fileName, PATHINFO_EXTENSION);
+            if ($fileExtension && !$this->checkAllowedExtension($fileExtension)) {
+                throw new \Magento\Framework\Exception\LocalizedException(__('Disallowed file type.'));
+            }
+
             $fileName = preg_replace('/[^a-z0-9\._-]+/i', '', $fileName);
             $this->_directory->writeFile(
-                $this->_directory->getRelativePath($this->getTmpDir() . '/' . $fileName),
+                $this->_directory->getRelativePath($filePath . $fileName),
                 $read->readAll()
             );
         }
 
-        $filePath = $this->_directory->getRelativePath($this->getTmpDir() . '/' . $fileName);
+        $filePath = $this->_directory->getRelativePath($filePath . $fileName);
         $this->_setUploadFile($filePath);
         $destDir = $this->_directory->getAbsolutePath($this->getDestDir());
         $result = $this->save($destDir);
@@ -332,7 +354,7 @@ protected function _moveFile($tmpPath, $destPath)
     }
 
     /**
-     * {@inheritdoc}
+     * @inheritdoc
      */
     protected function chmod($file)
     {

app/code/Magento/CatalogImportExport/Test/Unit/Model/Import/UploaderTest.php

@@ -105,17 +105,17 @@ protected function setUp()
                 null,
                 $this->directoryResolver,
             ])
-            ->setMethods(['_setUploadFile', 'save', 'getTmpDir'])
+            ->setMethods(['_setUploadFile', 'save', 'getTmpDir', 'checkAllowedExtension'])
             ->getMock();
     }
 
     /**
      * @dataProvider moveFileUrlDataProvider
      */
-    public function testMoveFileUrl($fileUrl, $expectedHost, $expectedFileName)
+    public function testMoveFileUrl($fileUrl, $expectedHost, $expectedFileName, $checkAllowedExtension)
     {
         $destDir = 'var/dest/dir';
-        $expectedRelativeFilePath = $this->uploader->getTmpDir() . '/' . $expectedFileName;
+        $expectedRelativeFilePath = $expectedFileName;
         $this->directoryMock->expects($this->once())->method('isWritable')->with($destDir)->willReturn(true);
         $this->directoryMock->expects($this->any())->method('getRelativePath')->with($expectedRelativeFilePath);
         $this->directoryMock->expects($this->once())->method('getAbsolutePath')->with($destDir)
@@ -140,6 +140,9 @@ public function testMoveFileUrl($fileUrl, $expectedHost, $expectedFileName)
         $this->uploader->expects($this->once())->method('_setUploadFile')->will($this->returnSelf());
         $this->uploader->expects($this->once())->method('save')->with($destDir . '/' . $expectedFileName)
             ->willReturn(['name' => $expectedFileName, 'path' => 'absPath']);
+        $this->uploader->expects($this->exactly($checkAllowedExtension))
+            ->method('checkAllowedExtension')
+            ->willReturn(true);
 
         $this->uploader->setDestDir($destDir);
         $result = $this->uploader->move($fileUrl);
@@ -151,7 +154,7 @@ public function testMoveFileName()
     {
         $destDir = 'var/dest/dir';
         $fileName = 'test_uploader_file';
-        $expectedRelativeFilePath = $this->uploader->getTmpDir() . '/' . $fileName;
+        $expectedRelativeFilePath = $fileName;
         $this->directoryMock->expects($this->once())->method('isWritable')->with($destDir)->willReturn(true);
         $this->directoryMock->expects($this->any())->method('getRelativePath')->with($expectedRelativeFilePath);
         $this->directoryMock->expects($this->once())->method('getAbsolutePath')->with($destDir)
@@ -176,11 +179,13 @@ public function moveFileUrlDataProvider()
                 '$fileUrl' => 'http://test_uploader_file',
                 '$expectedHost' => 'test_uploader_file',
                 '$expectedFileName' => 'httptest_uploader_file',
+                '$checkAllowedExtension' => 0
             ],
             [
                 '$fileUrl' => 'https://!:^&`;file',
                 '$expectedHost' => '!:^&`;file',
                 '$expectedFileName' => 'httpsfile',
+                '$checkAllowedExtension' => 0
             ],
         ];
     }