implement clamav scan on upload #782

.buildpacks

@@ -1 +1,2 @@
-https://github.com/Scalingo/php-buildpack
+https://github.com/Scalingo/clamav-buildpack
+https://github.com/Scalingo/php-buildpack

.docker/php-fpm/Dockerfile

@@ -26,7 +26,7 @@ RUN apt-get update && apt-get install -y --no-install-recommends \
         && pecl install redis \
         && docker-php-ext-enable redis \
         # Install others
-        && docker-php-ext-install -j2 bcmath ctype iconv pdo pdo_mysql pdo_sqlite zip xsl\
+        && docker-php-ext-install -j2 bcmath ctype iconv pdo pdo_mysql pdo_sqlite zip xsl sockets\
         && docker-php-ext-enable opcache \
         && rm /usr/src/php/ext/*.tgz \
         && curl -sS https://getcomposer.org/installer | php && mv composer.phar /usr/local/bin/composer

.env

@@ -30,7 +30,9 @@ MATOMO_SITE_ID=
 SESSION_MAXLIFETIME=151200 # 151200 secondes = 42 heures
 CONTACT_FORM_LIMITER_LIMIT=3
 CONTACT_FORM_LIMITER_INTERVAL='20 minutes'
-
+CLAMAV_HOST=stopunaises_clamav
+CLAMAV_STRATEGY=clamd_unix
+CLAMAV_SCAN_ENABLE=1
 ###> sentry/sentry-symfony ###
 SENTRY_DSN=
 ###< sentry/sentry-symfony ###

.env.sample

@@ -30,7 +30,9 @@ MATOMO_SITE_ID=
 SESSION_MAXLIFETIME=151200 # 151200 secondes = 42 heures
 CONTACT_FORM_LIMITER_LIMIT=3
 CONTACT_FORM_LIMITER_INTERVAL='20 minutes'
-
+CLAMAV_HOST=stopunaises_clamav
+CLAMAV_STRATEGY=clamd_unix
+CLAMAV_SCAN_ENABLE=1
 ###> sentry/sentry-symfony ###
 SENTRY_DSN=
 ###< sentry/sentry-symfony ###

.slugignore

@@ -3,4 +3,5 @@ docker-compose.yml
 .github
 tools
 cypress
-cypress.config.js
+cypress.config.js
+/node_modules

Makefile

@@ -173,3 +173,6 @@ tools-down: ## [Tools] Shutdown tools containers
 
 k6: ## Run K6 tests
 	@$(DOCKER_COMP) -f $(DOCKER_COMP_FILE_TOOLS) run --rm -T stopunaises_k6 run -<tools/k6/k6-50000.js --env "URL=$(URL)"
+
+clamscan-tmp: ## [Clamscan] Scan a tmp directory
+	@bash -l -c '$(DOCKER_COMP) exec -it stopunaises_clamav clamscan /app/tmp -r'

composer.json

@@ -16,6 +16,7 @@
         "phpdocumentor/reflection-docblock": "^5.3",
         "phpstan/phpdoc-parser": "^1.8",
         "sentry/sentry-symfony": "^4.5",
+        "sineflow/clamav": "^1.1",
         "symfony/asset": "7.1.*",
         "symfony/brevo-mailer": "7.1.*",
         "symfony/console": "7.1.*",

config/bundles.php

@@ -16,4 +16,5 @@
     League\FlysystemBundle\FlysystemBundle::class => ['all' => true],
     Sentry\SentryBundle\SentryBundle::class => ['prod' => true],
     DAMA\DoctrineTestBundle\DAMADoctrineTestBundle::class => ['test' => true],
+    Sineflow\ClamAV\Bundle\SineflowClamAVBundle::class => ['all' => true],
 ];

config/packages/clamav.yaml

@@ -0,0 +1,5 @@
+sineflow_clam_av:
+  strategy: '%env(CLAMAV_STRATEGY)%'
+  host: '%env(CLAMAV_HOST)%'
+  socket: "/app/run/clamd.sock"
+  port: 3310

docker-compose.yml

@@ -64,5 +64,10 @@ services:
     image: redis:7.0-alpine
     container_name: stopunaises_redis
 
+  stopunaises_clamav:
+    image: clamav/clamav
+    container_name: stopunaises_clamav
+    volumes:
+      - .:/app/
 volumes:
   dbdata:

scalingo.json

@@ -4,6 +4,14 @@
     "COMPOSER_DEV": {
       "description": "Install composer dev dependencies",
       "value": "true"
+    },
+    "CLAMD_DISABLE_DAEMON": {
+      "description": "When set, this environment variable instructs the image to NOT start the clamd daemon.",
+      "value": "true"
+    },
+    "FRESHCLAM_DISABLE_DAEMON": {
+      "description": "When set, this environment variable instructs the image to NOT start the freshclam daemon.",
+      "value": "true"
     }
   }
-}
+}

src/Entity/Signalement.php

@@ -152,7 +152,7 @@ class Signalement
     private ?array $punaisesDetails = null;
 
     #[ORM\Column(type: 'json', nullable: true)]
-    private $photos = [];
+    private array $photos = [];
 
     #[ORM\ManyToOne(inversedBy: 'signalements')]
     private ?Territoire $territoire = null;
@@ -194,15 +194,15 @@ class Signalement
     private ?string $uuidPublic = null;
 
     #[ORM\Column(type: 'json')]
-    private $geoloc = [];
+    private array $geoloc = [];
 
     #[ORM\Column(type: 'string', enumType: SignalementType::class)]
     private SignalementType $type;
 
     #[ORM\Column(nullable: true)]
     private ?\DateTimeImmutable $punaisesViewedAt = null;
 
-    #[ORM\Column(type: 'string', enumType: PlaceType::class, nullable: true)]
+    #[ORM\Column(type: 'string', nullable: true, enumType: PlaceType::class)]
     private ?PlaceType $placeType = null;
 
     #[ORM\Column(length: 50, nullable: true)]

src/Exception/File/MalwareDetectedException.php

@@ -0,0 +1,11 @@
+<?php
+
+namespace App\Exception\File;
+
+class MalwareDetectedException extends \Exception
+{
+    public function __construct(string $filename)
+    {
+        parent::__construct(\sprintf('Le fichier %s est infecté ', $filename));
+    }
+}

src/Security/FileScanner.php

@@ -0,0 +1,42 @@
+<?php
+
+namespace App\Security;
+
+use Sineflow\ClamAV\Exception\FileScanException;
+use Sineflow\ClamAV\Exception\SocketException;
+use Sineflow\ClamAV\Scanner;
+use Symfony\Component\DependencyInjection\Attribute\Autowire;
+use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
+use Symfony\Component\Uid\Uuid;
+
+readonly class FileScanner
+{
+    public function __construct(
+        private Scanner $scanner,
+        private ParameterBagInterface $parameterBag,
+        #[Autowire(env: 'CLAMAV_SCAN_ENABLE')]
+        private bool $clamavScanEnable,
+    ) {
+    }
+
+    /**
+     * @throws FileScanException
+     * @throws SocketException
+     */
+    public function isClean(string $filePath, ?bool $copy = true): bool
+    {
+        if (!$this->clamavScanEnable) {
+            return true;
+        }
+        if ($copy) {
+            $copiedFilepath = $this->parameterBag->get('uploads_tmp_dir').'clamav_'.Uuid::v4();
+            file_put_contents($copiedFilepath, file_get_contents($filePath));
+        } else {
+            $copiedFilepath = $filePath;
+        }
+
+        $scannedFile = $this->scanner->scan($copiedFilepath);
+
+        return $scannedFile->isClean();
+    }
+}

src/Service/Upload/UploadHandlerService.php

@@ -2,74 +2,42 @@
 
 namespace App\Service\Upload;
 
+use App\Exception\File\MalwareDetectedException;
 use App\Exception\File\MaxUploadSizeExceededException;
+use App\Security\FileScanner;
 use League\Flysystem\FilesystemException;
 use League\Flysystem\FilesystemOperator;
 use Psr\Log\LoggerInterface;
-use Symfony\Component\DependencyInjection\ParameterBag\ParameterBagInterface;
-use Symfony\Component\HttpFoundation\File\Exception\FileException;
 use Symfony\Component\HttpFoundation\File\UploadedFile;
 use Symfony\Component\String\Slugger\SluggerInterface;
 
 class UploadHandlerService
 {
-    public const MAX_FILESIZE = 10 * 1024 * 1024;
+    public const int|float MAX_FILESIZE = 10 * 1024 * 1024;
     public const UPLOAD_ACCEPTED_EXTENSIONS = ['jpg', 'jpeg', 'png'];
     public const UPLOAD_ACCEPTED_MIME_TYPES = ['image/jpeg', 'image/png'];
 
-    private $file;
+    private ?array $file;
 
     public function __construct(
-        private FilesystemOperator $fileStorage,
-        private ParameterBagInterface $parameterBag,
-        private SluggerInterface $slugger,
-        private LoggerInterface $logger,
+        private readonly FilesystemOperator $fileStorage,
+        private readonly SluggerInterface $slugger,
+        private readonly LoggerInterface $logger,
+        private readonly FileScanner $fileScanner,
     ) {
         $this->file = null;
     }
 
-    public function toTempFolder(UploadedFile $file): self|array
+    /**
+     * @throws MaxUploadSizeExceededException
+     * @throws MalwareDetectedException
+     */
+    public function uploadFromFile(UploadedFile $file, $newFilename): void
     {
-        $originalFilename = pathinfo($file->getClientOriginalName(), \PATHINFO_FILENAME);
-        $titre = $originalFilename.'.'.$file->guessExtension();
-        // this is needed to safely include the file name as part of the URL
-        $safeFilename = $this->slugger->slug($originalFilename);
-        $newFilename = $safeFilename.'-'.uniqid().'.'.$file->guessExtension();
-        if ($file->getSize() > self::MAX_FILESIZE) {
-            throw new MaxUploadSizeExceededException(self::MAX_FILESIZE);
-        }
-        try {
-            $file->move(
-                $this->parameterBag->get('uploads_tmp_dir'),
-                $newFilename
-            );
-        } catch (FileException $e) {
-            $this->logger->error($e->getMessage());
-
-            return ['error' => 'Erreur lors du téléversement.', 'message' => $e->getMessage(), 'status' => 500];
+        if (!$this->fileScanner->isClean($file->getPathname())) {
+            throw new MalwareDetectedException($file->getClientOriginalName());
         }
-        $this->file = ['file' => $newFilename, 'titre' => $titre];
 
-        return $this;
-    }
-
-    public function uploadFromFilename(string $filename): string
-    {
-        $tmpFilepath = $this->parameterBag->get('uploads_tmp_dir').$filename;
-
-        try {
-            $resourceFile = fopen($tmpFilepath, 'r');
-            $this->fileStorage->writeStream($filename, $resourceFile);
-            fclose($resourceFile);
-        } catch (FilesystemException $exception) {
-            $this->logger->error($exception->getMessage());
-        }
-
-        return $filename;
-    }
-
-    public function uploadFromFile(UploadedFile $file, $newFilename): void
-    {
         if ($file->getSize() > self::MAX_FILESIZE) {
             throw new MaxUploadSizeExceededException(self::MAX_FILESIZE);
         }
@@ -82,12 +50,15 @@ public function uploadFromFile(UploadedFile $file, $newFilename): void
         }
     }
 
+    /**
+     * @throws FilesystemException
+     */
     public function createTmpFileFromBucket($from, $to): void
     {
         $resourceBucket = $this->fileStorage->read($from);
-        $resourceFileSytem = fopen($to, 'w');
-        fwrite($resourceFileSytem, $resourceBucket);
-        fclose($resourceFileSytem);
+        $resourceFileSystem = fopen($to, 'w');
+        fwrite($resourceFileSystem, $resourceBucket);
+        fclose($resourceFileSystem);
     }
 
     public function setKey(string $key): ?array
@@ -116,7 +87,7 @@ public function handleUploadFilesRequest(
         ?array $filesPosted,
     ): array {
         $filesToSave = [];
-        if (isset($filesPosted) && \is_array($filesPosted)) {
+        if (isset($filesPosted)) {
             /** @var UploadedFile $file */
             foreach ($filesPosted as $file) {
                 if ($file->getError()) {
@@ -134,12 +105,9 @@ public function handleUploadFilesRequest(
                 $newFilename = $safeFilename.'-'.uniqid().'.'.$file->guessExtension();
                 try {
                     $this->uploadFromFile($file, $newFilename);
-                } catch (FilesystemException $exception) {
-                    $newFilename = '';
-                    $this->logger->error($exception->getMessage());
-                } catch (MaxUploadSizeExceededException $exception) {
+                } catch (MaxUploadSizeExceededException|MalwareDetectedException $exception) {
                     $newFilename = '';
-                    $this->logger->error($exception->getMessage());
+                    $this->logger->error($errorMessage = $exception->getMessage());
                 }
                 if (!empty($newFilename)) {
                     $filesToSave[] = [