Remote Code Execution & File Deletion in Asset Uploads

@@ -67,7 +67,9 @@
                 'arguments' => 'mautic.factory',
             ],
             // Override the DropzoneController
-            'oneup_uploader.controller.dropzone.class' => Mautic\AssetBundle\Controller\UploadController::class,
+            'oneup_uploader.controller.dropzone.class' => [
+                'class'     => Mautic\AssetBundle\Controller\UploadController::class,
+            ],
         ],
         'fixtures' => [
             'mautic.asset.fixture.asset' => [
@@ -82,5 +84,30 @@
         'max_size'            => '6',
         'allowed_extensions'  => ['csv', 'doc', 'docx', 'epub', 'gif', 'jpg', 'jpeg', 'mpg', 'mpeg', 'mp3', 'odt', 'odp', 'ods', 'pdf', 'png', 'ppt', 'pptx', 'tif', 'tiff', 'txt', 'xls', 'xlsx', 'wav'],
         'streamed_extensions' => ['gif', 'jpg', 'jpeg', 'mpg', 'mpeg', 'mp3', 'pdf', 'png', 'wav'],
+        'allowed_mimetypes'   => [
+            'csv'  => 'text/csv',
+            'doc'  => 'application/msword',
+            'docx' => 'application/vnd.openxmlformats-officedocument.wordprocessingml.document',
+            'epub' => 'application/epub+zip',
+            'gif'  => 'image/gif',
+            'jpg'  => 'image/jpeg',
+            'jpeg' => 'image/jpeg',
+            'mpg'  => 'video/mpeg',
+            'mpeg' => 'video/mpeg',
+            'mp3'  => 'audio/mpeg',
+            'odt'  => 'application/vnd.oasis.opendocument.text',
+            'odp'  => 'application/vnd.oasis.opendocument.presentation',
+            'ods'  => 'application/vnd.oasis.opendocument.spreadsheet',
+            'pdf'  => 'application/pdf',
+            'png'  => 'image/png',
+            'ppt'  => 'application/vnd.ms-powerpoint',
+            'pptx' => 'application/vnd.openxmlformats-officedocument.presentationml.presentation',
+            'tif'  => 'image/tiff',
+            'tiff' => 'image/tiff',
+            'txt'  => 'text/plain',
+            'xls'  => 'application/vnd.ms-excel',
+            'xlsx' => 'application/vnd.openxmlformats-officedocument.spreadsheetml.sheet',
+            'wav'  => 'audio/wav',
+        ],
     ],
 ];

@@ -17,6 +17,7 @@ public function upload(): JsonResponse
         $request  = $this->getRequest();
         $response = new EmptyResponse();
         $files    = $this->getFiles($request->files);
+        $this->setTranslator($this->container->get('translator'));
 
         if (!empty($files)) {
             foreach ($files as $file) {
@@ -26,7 +27,6 @@ public function upload(): JsonResponse
                     $this->errorHandler->addException($response, $e);
                 } catch (\Exception $e) {
                     error_log($e);
-
                     $error = new UploadException($this->translator->trans('mautic.asset.error.file.failed'));
                     $this->errorHandler->addException($response, $error);
                 }

@@ -8,6 +8,7 @@
 use Mautic\CoreBundle\Doctrine\Mapping\ClassMetadataBuilder;
 use Mautic\CoreBundle\Entity\FormEntity;
 use Mautic\CoreBundle\Helper\FileHelper;
+use Mautic\CoreBundle\Loader\ParameterLoader;
 use Symfony\Component\Filesystem\Filesystem;
 use Symfony\Component\HttpFoundation\File\Exception\FileNotFoundException;
 use Symfony\Component\HttpFoundation\File\File;
@@ -1164,6 +1165,29 @@ public static function validateFile($object, ExecutionContextInterface $context)
                     ->setTranslationDomain('validators')
                     ->addViolation();
             }
+            $loader           = new ParameterLoader();
+            $parameters       = $loader->getParameterBag();
+            $mimeTypesAllowed = $parameters->get('allowed_mimetypes');
+
+            if (!empty($object->getFileMimeType()) && !in_array($object->getFileMimeType(), $mimeTypesAllowed)) {
+                $context->buildViolation('mautic.asset.asset.error.invalid.mimetype', [
+                    '%fileMimetype%'=> $object->getFileMimeType(),
+                    '%mimetypes%'   => implode(', ', $mimeTypesAllowed),
+                ])->atPath('file')
+                    ->setTranslationDomain('validators')
+                    ->addViolation();
+            }
+
+            $extensionsAllowed = array_keys($mimeTypesAllowed);
+            $fileType          = $object->getExtension();
+            if (null !== $object->getExtension() && !in_array($fileType, $extensionsAllowed)) {
+                $context->buildViolation('mautic.asset.asset.error.file.extension', [
+                    '%fileExtension%'=> $object->getExtension(),
+                    '%extensions%'   => implode(', ', $extensionsAllowed),
+                ])->atPath('file')
+                    ->setTranslationDomain('validators')
+                    ->addViolation();
+            }
 
             // Unset any remote file data
             $object->setRemotePath(null);

@@ -5,6 +5,7 @@
 use Mautic\AssetBundle\Model\AssetModel;
 use Mautic\CoreBundle\Exception\FileInvalidException;
 use Mautic\CoreBundle\Helper\CoreParametersHelper;
+use Mautic\CoreBundle\Translation\Translator;
 use Mautic\CoreBundle\Validator\FileUploadValidator;
 use Oneup\UploaderBundle\Event\PostUploadEvent;
 use Oneup\UploaderBundle\Event\ValidationEvent;
@@ -17,7 +18,8 @@ class UploadSubscriber implements EventSubscriberInterface
     public function __construct(
         private CoreParametersHelper $coreParametersHelper,
         private AssetModel $assetModel,
-        private FileUploadValidator $fileUploadValidator
+        private FileUploadValidator $fileUploadValidator,
+        protected Translator $translator,
     ) {
     }
 
@@ -59,7 +61,8 @@ public function onPostUpload(PostUploadEvent $event): void
     public function onUploadValidation(ValidationEvent $event): void
     {
         $file       = $event->getFile();
-        $extensions = $this->coreParametersHelper->get('allowed_extensions');
+        $mimetypes  = $this->coreParametersHelper->get('allowed_mimetypes');
+        $extensions = array_keys($mimetypes);
         $maxSize    = $this->assetModel->getMaxUploadSize('B');
 
         if (null === $file) {
@@ -77,5 +80,26 @@ public function onUploadValidation(ValidationEvent $event): void
         } catch (FileInvalidException $e) {
             throw new ValidationException($e->getMessage());
         }
+
+        try {
+            $this->checkMimeType($file->getMimeType(), $mimetypes, 'mautic.asset.asset.error.file.mimetype');
+        } catch (FileInvalidException $e) {
+            throw new ValidationException($e->getMessage());
+        }
+    }
+
+    /**
+     * @param array<string,string> $allowedMimeTypes
+     */
+    private function checkMimeType(string $mimeType, array $allowedMimeTypes, string $extensionErrorMsg): void
+    {
+        if (!in_array(strtolower($mimeType), array_map('strtolower', $allowedMimeTypes), true)) {
+            $error = $this->translator->trans($extensionErrorMsg, [
+                '%fileMimetype%' => $mimeType,
+                '%mimetypes%'    => implode(', ', $allowedMimeTypes),
+            ], 'validators');
+
+            throw new FileInvalidException($error);
+        }
     }
 }

@@ -12,15 +12,18 @@
 use Mautic\CoreBundle\Form\Type\PublishDownDateType;
 use Mautic\CoreBundle\Form\Type\PublishUpDateType;
 use Mautic\CoreBundle\Form\Type\YesNoButtonGroupType;
+use Mautic\CoreBundle\Loader\ParameterLoader;
 use Symfony\Component\Form\AbstractType;
 use Symfony\Component\Form\Extension\Core\Type\HiddenType;
 use Symfony\Component\Form\Extension\Core\Type\LocaleType;
 use Symfony\Component\Form\Extension\Core\Type\TextareaType;
 use Symfony\Component\Form\Extension\Core\Type\TextType;
 use Symfony\Component\Form\FormBuilderInterface;
 use Symfony\Component\OptionsResolver\OptionsResolver;
+use Symfony\Component\Validator\Constraints\Callback;
 use Symfony\Component\Validator\Constraints\NotBlank;
 use Symfony\Component\Validator\Constraints\Url;
+use Symfony\Component\Validator\Context\ExecutionContextInterface;
 use Symfony\Contracts\Translation\TranslatorInterface;
 
 /**
@@ -55,18 +58,24 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
             'tempName',
             HiddenType::class,
             [
-                'label'      => $this->translator->trans('mautic.asset.asset.form.file.upload', ['%max%' => $maxUploadSize]),
-                'label_attr' => ['class' => 'control-label'],
-                'required'   => false,
+                'label'       => $this->translator->trans('mautic.asset.asset.form.file.upload', ['%max%' => $maxUploadSize]),
+                'label_attr'  => ['class' => 'control-label'],
+                'required'    => false,
+                'constraints' => [
+                    new Callback([$this, 'validateExtension']),
+                ],
             ]
         );
 
         $builder->add(
             'originalFileName',
             HiddenType::class,
             [
-                'required' => false,
-            ]
+                'required'    => false,
+                'constraints' => [
+                    new Callback([$this, 'validateExtension']),
+                ],
+            ],
         );
         $builder->add(
             'disallow',
@@ -180,6 +189,33 @@ public function buildForm(FormBuilderInterface $builder, array $options): void
         }
     }
 
+    /**
+     * @param Asset|string|null $object
+     */
+    public function validateExtension($object, ExecutionContextInterface $context): void
+    {
+        if (empty($object)) {
+            return;
+        }
+        $parameters       = (new ParameterLoader())->getParameterBag();
+        $mimeTypesAllowed = $parameters->get('allowed_mimetypes');
+        $extensions       = array_keys($mimeTypesAllowed);
+        $fileName         = $object;
+        if (!is_string($object) && $object instanceof Asset) {
+            $fileName = $object->getOriginalFileName();
+        }
+        $fileExtension    = pathinfo($fileName, PATHINFO_EXTENSION);
+        if (!in_array($fileExtension, $extensions, true)) {
+            $context->buildViolation('mautic.asset.asset.error.file.extension', [
+                '%fileExtension%'=> $fileExtension,
+                '%extensions%'   => implode(', ', $extensions),
+            ])
+                ->atPath('file')
+                ->setTranslationDomain('validators')
+                ->addViolation();
+        }
+    }
+
     public function configureOptions(OptionsResolver $resolver): void
     {
         $resolver->setDefaults(['data_class' => Asset::class]);

@@ -233,4 +233,63 @@ private function setPermission(User $user, array $permissions): void
         $this->em->persist($role);
         $this->em->flush();
     }
+
+    public function testPostRequestWithWrongTempNameAndOriginalFileNameFileExtension(): void
+    {
+        $response = $this->client->request(
+            Request::METHOD_GET,
+            '/s/assets/new',
+        );
+        $this->assertResponseStatusCodeSame(Response::HTTP_OK);
+        $form                              = $response->filter('form[name="asset"]')->form();
+        $data                              = $form->getPhpValues();
+        $data['asset']['tempName']         = 'image2.php';
+        $data['asset']['originalFileName'] = 'originalImage2.php';
+        $data['asset']['storageLocation']  = 'local';
+        $data['asset']['title']            = 'title';
+        $data['asset']['description']      = 'description';
+        $this->client->submit($form, $data);
+        preg_match_all('/Upload failed as the file extension, php/', $this->client->getResponse()->getContent(), $matches);
+        $this->assertCount(2, $matches[0]);
+        $this->assertStringContainsString('Upload failed as the file extension, php', $this->client->getResponse()->getContent());
+    }
+
+    public function testPostRequestWithWrongTempNameFileExtension(): void
+    {
+        $response = $this->client->request(
+            Request::METHOD_GET,
+            '/s/assets/new',
+        );
+        $this->assertResponseStatusCodeSame(Response::HTTP_OK);
+        $form                              = $response->filter('form[name="asset"]')->form();
+        $data                              = $form->getPhpValues();
+        $data['asset']['tempName']         = 'image2.php';
+        $data['asset']['originalFileName'] = 'originalImage2.png';
+        $data['asset']['storageLocation']  = 'local';
+        $data['asset']['title']            = 'title';
+        $data['asset']['description']      = 'description';
+        $this->client->submit($form, $data);
+        preg_match_all('/Upload failed as the file extension, php/', $this->client->getResponse()->getContent(), $matches);
+        $this->assertCount(1, $matches[0]);
+        $this->assertStringContainsString('Upload failed as the file extension, php', $this->client->getResponse()->getContent());
+    }
+
+    public function testPostResquetSuccessWithCorrectFileExtension(): void
+    {
+        $response = $this->client->request(
+            Request::METHOD_GET,
+            '/s/assets/new',
+        );
+        $this->assertResponseStatusCodeSame(Response::HTTP_OK);
+        $form                              = $response->filter('form[name="asset"]')->form();
+        $data                              = $form->getPhpValues();
+        $data['asset']['tempName']         = 'image.png';
+        $data['asset']['originalFileName'] = 'originalImage.png';
+        $data['asset']['storageLocation']  = 'local';
+        $data['asset']['title']            = 'title';
+        $data['asset']['description']      = 'description';
+        $this->client->submit($form, $data);
+        $this->assertResponseStatusCodeSame(Response::HTTP_OK);
+        $this->assertStringNotContainsString('Upload failed as the file extension, php', $this->client->getResponse()->getContent());
+    }
 }

@@ -0,0 +1,114 @@
+<?php
+
+namespace Mautic\AssetBundle\Tests\Controller;
+
+use Mautic\AssetBundle\Tests\Asset\AbstractAssetTest;
+use Symfony\Component\HttpFoundation\File\UploadedFile;
+use Symfony\Component\HttpFoundation\Request;
+use Symfony\Component\HttpFoundation\Response;
+
+class UploadControllerFunctionalTest extends AbstractAssetTest
+{
+    public function testUploadWithWrongMimetype(): void
+    {
+        // Create a php file with the content of phpinfo
+        $assetsPath = $this->client->getKernel()->getContainer()->getParameter('mautic.upload_dir');
+
+        $fileName = 'image2.png';
+        $filePath = $assetsPath.'/'.$fileName;
+
+        if (file_exists($filePath)) {
+            unlink($filePath);
+        }
+
+        copy('index.php', $filePath);
+
+        $binaryFile = new UploadedFile($filePath, $fileName, 'application/x-httpd-php', null, true);
+
+        $tmpId = 'tempId_'.time();
+        // Upload the file
+        $this->client->request(
+            Request::METHOD_POST,
+            '/s/_uploader/asset/upload',
+            [
+                'tempId' => $tmpId,
+            ],
+            [
+                'file' => $binaryFile,
+            ]
+        );
+
+        $response = $this->client->getResponse();
+        $this->assertStringContainsString('Upload failed as the file mimetype', $response->getContent());
+        $this->assertStringContainsString('text\/x-php is not allowed', $response->getContent());
+        unlink($filePath);
+    }
+
+    public function testSuccessUploadWithPng(): void
+    {
+        // Create a temporary PNG file
+        // Create a php file with the content of phpinfo
+        $assetsPath     = $this->client->getKernel()->getContainer()->getParameter('mautic.upload_dir');
+        $assetsPathFrom = $this->client->getKernel()->getContainer()->getParameter('mautic.application_dir').'/app/assets/images/mautic_logo_db64.png';
+
+        $fileName = 'image3.png';
+        $filePath = $assetsPath.'/'.$fileName;
+
+        copy($assetsPathFrom, $filePath);
+        // Create an UploadedFile instance with the correct MIME type
+        $uploadedFile = new UploadedFile($filePath, $fileName, 'image/png', null, true);
+
+        $tmpId = 'tempId_'.time();
+        // Perform the request with the file
+        $this->client->request(
+            'POST',
+            '/s/_uploader/asset/upload',
+            ['tempId' => $tmpId],
+            ['file'   => $uploadedFile]
+        );
+        $this->assertResponseStatusCodeSame(Response::HTTP_OK);
+        $this->assertStringContainsString('state":1', $this->client->getResponse()->getContent());
+        if (file_exists($filePath)) {
+            unlink($filePath);
+        }
+        $data = json_decode($this->client->getResponse()->getContent(), true);
+        unlink($assetsPath.'/tmp/'.$tmpId.'/'.$data['tmpFileName']);
+        rmdir($assetsPath.'/tmp/'.$tmpId);
+    }
+
+    public function testUploadWithWrongExtension(): void
+    {
+        // Create a php file with the content of phpinfo
+        $assetsPath     = $this->client->getKernel()->getContainer()->getParameter('mautic.upload_dir');
+        $assetsPathFrom = $this->client->getKernel()->getContainer()->getParameter('mautic.application_dir').'/app/assets/images/mautic_logo_db64.png';
+
+        $fileName = 'image2.php';
+        $filePath = $assetsPath.'/'.$fileName;
+
+        if (file_exists($filePath)) {
+            unlink($filePath);
+        }
+
+        copy($assetsPathFrom, $filePath);
+
+        $binaryFile = new UploadedFile($filePath, $fileName, 'image/png', null, true);
+
+        $tmpId = 'tempId_'.time();
+        // Upload the file
+        $this->client->request(
+            Request::METHOD_POST,
+            '/s/_uploader/asset/upload',
+            [
+                'tempId' => $tmpId,
+            ],
+            [
+                'file' => $binaryFile,
+            ]
+        );
+
+        $response = $this->client->getResponse();
+        $this->assertStringContainsString('Upload failed as the file extension', $response->getContent());
+        $this->assertStringContainsString('Upload failed as the file extension, php,', $response->getContent());
+        unlink($filePath);
+    }
+}

@@ -91,3 +91,5 @@ mautic.widget.unique.vs.repetitive.downloads="Unique vs repetitive downloads"
 mautic.widget.popular.assets="Popular assets"
 mautic.widget.created.assets="Created assets"
 mautic.report.group.assets="Assets"
+mautic.asset.asset.help.searchcommands="<strong>Search commands</strong><br />ids:ID1,ID2 (comma separated IDs, no spaces)<br />is:mine<br />is:published<br />is:unpublished<br />name:*<br />is:uncategorized<br />category:{category alias}"
+mautic.asset.asset.error.file.mimetype="The file type is not allowed."
\ No newline at end of file

@@ -4,4 +4,6 @@ mautic.asset.asset.error.missing.remote.path="A remote URL must be specified whe
 mautic.asset.asset.error.file.size="Upload failed as the file is %fileSize% MB which exceeds the maximum allowed file size of %maxSize% MB. This setting can be changed in the Configuration."
 mautic.asset.asset.error.file.extension="Upload failed as the file extension, %fileExtension%, is not in the list of allowed extensions (%extensions%). This setting can be changed in the Configuration."
 mautic.asset.asset.error.file.extension.js="Upload failed as the file extension is not in the list of allowed extensions (%extensions%). This setting can be changed in the Configuration."
-mautic.asset.validation.error.url="The remote should be a valid URL."
\ No newline at end of file
+mautic.asset.validation.error.url="The remote should be a valid URL."
+mautic.asset.asset.error.file.mimetype="Upload failed as the file mimetype, %fileMimetype% is not allowed. Allowed file types are %mimetypes%."
+mautic.asset.asset.error.invalid.mimetype="Upload failed as the file mimetype, %fileMimetype% is not allowed. Allowed file types are %mimetypes%."
\ No newline at end of file