From 579f54a42ffa792ea782af75f9bcd9431dfc4835 Mon Sep 17 00:00:00 2001 From: Ronan Lenouvel Date: Sun, 11 Oct 2026 18:42:22 +0200 Subject: [PATCH] =?UTF-8?q?=F0=9F=94=92=20security(ImageSanitizer):=20imag?= =?UTF-8?q?es=20r=C3=A9-encod=C3=A9es=20sans=20m=C3=A9tadonn=C3=A9es=20ni?= =?UTF-8?q?=20octets=20parasites,=20plafond=20de=20volume=20par=20groupe?= =?UTF-8?q?=20contr=C3=B4l=C3=A9=20avant=20l'=C3=A9criture=20(#432)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .claude/architecture.md | 3 + composer.json | 4 + .../GroupDocumentRepositoryInterface.php | 3 + .../MysqlGroupDocumentRepository.php | 8 ++ src/Group/Service/GroupDocumentService.php | 41 +++++- src/Group/Service/ImageSanitizer.php | 85 +++++++++++ .../Service/GroupDocumentServiceTest.php | 132 +++++++++++++++++- 7 files changed, 269 insertions(+), 7 deletions(-) create mode 100644 src/Group/Service/ImageSanitizer.php diff --git a/.claude/architecture.md b/.claude/architecture.md index a89b056d..672d84c6 100644 --- a/.claude/architecture.md +++ b/.claude/architecture.md @@ -119,6 +119,9 @@ Politique unique dans `App\Security\SecurityHeaders`, appliquée par le Kernel - **Pas de fichier orphelin** : si le quota est atteint ou si l'enregistrement échoue, le fichier copié est retiré ; à la suppression d'un document, la ligne part d'abord puis le fichier ; à la suppression d'un **groupe**, ses fichiers sont listés avant (`GroupFilesPurgerInterface::filesOf`) puis retirés après (`remove`, au mieux) — `NoGroupFilesPurger` (Null Object) pour les scripts et tests. - **Nom d'origine borné** : sans chemin ni caractère de contrôle, 150 caractères au plus (extension conservée, jamais coupé au milieu d'un caractère), `document` s'il ne reste rien. +- **Images ré-encodées (#432)** : `Group\Service\ImageSanitizer` décode puis ré-encode chaque JPEG/PNG avec GD avant stockage : les octets ajoutés après l'image (fichier polyglotte), les segments cachés et les métadonnées (dont la position GPS d'une photo) disparaissent ; l'orientation EXIF d'une photo de téléphone est appliquée avant de la retirer. Dimensions déclarées contrôlées **avant** le décodage (12 000 px de côté, 40 millions de pixels : sinon `InvalidUploadException`). Les PDF sont stockés tels quels. Sans l'extension GD, l'image est copiée telle quelle (`composer.json` la suggère) : à vérifier sur le serveur. +- **Plafond de volume (#432)** : en plus du nombre (20), 100 Mo par groupe (`GroupDocumentService::MAX_BYTES_PER_GROUP`, `totalBytesByGroup`). Contrôlé une première fois AVANT d'écrire sur le disque, puis dans la transaction qui verrouille le groupe (c'est ce second contrôle qui fait foi). La taille enregistrée est celle du fichier réellement stocké (après ré-encodage). + ## Demandes de créneau : intégrité (#221) - **Date épinglée à la réponse** : « Accepter » ou « Refuser » envoie la date que le titulaire a sous les yeux (`occurrenceDate`, portée par `data-occurrence-date` sur les boutons) et l'`UPDATE` atomique ne s'applique que si la demande la porte encore (`respond(..., $expectedOccurrenceDate)`). Si le groupe demandeur l'a changée entre-temps : `RequestChangedException` → **409**, la demande reste en attente et le titulaire la revoit avec sa nouvelle date. `AvailabilityService::respond` et l'API **exigent** cette date (le paramètre est optionnel seulement dans le dépôt, pour les données de test). diff --git a/composer.json b/composer.json index 6b8481e7..fbf3e4c8 100644 --- a/composer.json +++ b/composer.json @@ -30,5 +30,9 @@ }, "config": { "sort-packages": true + }, + "suggest": { + "ext-gd": "Ré-encodage des images envoyées (#432) : sans elle, JPEG et PNG sont stockés tels quels", + "ext-exif": "Orientation des photos de téléphone appliquée avant de retirer les métadonnées (#432)" } } diff --git a/src/Group/Repository/GroupDocumentRepositoryInterface.php b/src/Group/Repository/GroupDocumentRepositoryInterface.php index aff94fbc..fd88c13c 100644 --- a/src/Group/Repository/GroupDocumentRepositoryInterface.php +++ b/src/Group/Repository/GroupDocumentRepositoryInterface.php @@ -15,6 +15,9 @@ public function findByGroup(int $groupId): array; public function countByGroup(int $groupId): int; + /** Volume total des documents d'un groupe, en octets (plafond de volume, #432). */ + public function totalBytesByGroup(int $groupId): int; + /** * Verrouille le groupe jusqu'à la fin de la transaction en cours (#222) : les envois d'un MÊME groupe passent l'un après * l'autre, ce qui rend atomique « compter puis ajouter » (le quota ne peut plus être dépassé par des envois simultanés). diff --git a/src/Group/Repository/MysqlGroupDocumentRepository.php b/src/Group/Repository/MysqlGroupDocumentRepository.php index b87bb437..d5825209 100644 --- a/src/Group/Repository/MysqlGroupDocumentRepository.php +++ b/src/Group/Repository/MysqlGroupDocumentRepository.php @@ -44,6 +44,14 @@ public function countByGroup(int $groupId): int return (int) $statement->fetchColumn(); } + public function totalBytesByGroup(int $groupId): int + { + $statement = $this->pdo->prepare('SELECT COALESCE(SUM(size_bytes), 0) FROM group_documents WHERE group_id = :group_id'); + $statement->execute(['group_id' => $groupId]); + + return (int) $statement->fetchColumn(); + } + public function save(GroupDocument $document): GroupDocument { $statement = $this->pdo->prepare( diff --git a/src/Group/Service/GroupDocumentService.php b/src/Group/Service/GroupDocumentService.php index 6d42f8e1..4d2dec01 100644 --- a/src/Group/Service/GroupDocumentService.php +++ b/src/Group/Service/GroupDocumentService.php @@ -17,6 +17,8 @@ final class GroupDocumentService implements GroupFilesPurgerInterface { private const MAX_SIZE_BYTES = 10 * 1024 * 1024; + /** Volume total par groupe (#432) : le nombre de fichiers seul laisserait 20 × 10 Mo sur un hébergement au disque compté. */ + public const MAX_BYTES_PER_GROUP = 100 * 1024 * 1024; /** Longueur maximale du nom d'origine conservé (la colonne en accepte 255) : borné avant la base, jamais une erreur SQL. */ private const MAX_NAME_LENGTH = 150; @@ -33,6 +35,8 @@ public function __construct( private readonly TransactionRunner $transactions, private readonly string $storagePath, private readonly int $maxDocumentsPerGroup, + private readonly int $maxBytesPerGroup = self::MAX_BYTES_PER_GROUP, + private readonly ImageSanitizer $images = new ImageSanitizer(), ) { } @@ -51,6 +55,9 @@ public function upload(int $groupId, int $actorUserId, string $tmpFilePath, stri throw new InvalidUploadException('Type de fichier non autorisé (PDF, JPEG, PNG uniquement).'); } + // Avant toute écriture sur le disque : un groupe déjà plein ne fait pas copier un fichier pour rien (verdict définitif plus bas). + $this->assertRoomFor($groupId, $declaredSize); + $storedName = bin2hex(random_bytes(16)) . '.' . self::ALLOWED_MIME_TYPES[$realMimeType]; $destinationPath = $this->storagePath . '/' . $storedName; @@ -58,8 +65,18 @@ public function upload(int $groupId, int $actorUserId, string $tmpFilePath, stri throw new InvalidUploadException('Impossible de préparer le stockage des documents.'); } - if (!copy($tmpFilePath, $destinationPath)) { - throw new InvalidUploadException("Échec de l'enregistrement du fichier."); + try { + if ($realMimeType === 'application/pdf') { + if (!copy($tmpFilePath, $destinationPath)) { + throw new InvalidUploadException("Échec de l'enregistrement du fichier."); + } + } else { + $this->images->reencode($tmpFilePath, $realMimeType, $destinationPath); + } + } catch (\Throwable $e) { + @unlink($destinationPath); + + throw $e; } try { @@ -67,9 +84,7 @@ public function upload(int $groupId, int $actorUserId, string $tmpFilePath, stri // simultanés ne peuvent pas dépasser le quota. return $this->transactions->run(function () use ($groupId, $actorUserId, $originalName, $storedName, $realMimeType, $destinationPath): GroupDocument { $this->documentRepository->lockGroupQuota($groupId); - if ($this->documentRepository->countByGroup($groupId) >= $this->maxDocumentsPerGroup) { - throw new StorageQuotaExceededException('Nombre maximum de documents atteint pour ce groupe.'); - } + $this->assertRoomFor($groupId, filesize($destinationPath)); return $this->documentRepository->save(new GroupDocument( 0, @@ -89,6 +104,22 @@ public function upload(int $groupId, int $actorUserId, string $tmpFilePath, stri } } + /** + * Place pour un document de $bytes octets : nombre ET volume du groupe. Appelé une première fois avant d'écrire, puis dans la + * transaction qui verrouille le groupe (c'est ce second contrôle qui fait foi face à des envois simultanés). + * + * @throws StorageQuotaExceededException + */ + private function assertRoomFor(int $groupId, int $bytes): void + { + if ($this->documentRepository->countByGroup($groupId) >= $this->maxDocumentsPerGroup) { + throw new StorageQuotaExceededException('Nombre maximum de documents atteint pour ce groupe.'); + } + if ($this->documentRepository->totalBytesByGroup($groupId) + $bytes > $this->maxBytesPerGroup) { + throw new StorageQuotaExceededException('Le volume maximal de documents est atteint pour ce groupe.'); + } + } + /** @return list */ public function listForGroup(int $groupId, int $actorUserId): array { diff --git a/src/Group/Service/ImageSanitizer.php b/src/Group/Service/ImageSanitizer.php new file mode 100644 index 00000000..de01f723 --- /dev/null +++ b/src/Group/Service/ImageSanitizer.php @@ -0,0 +1,85 @@ + self::MAX_SIDE || $size[1] > self::MAX_SIDE || $size[0] * $size[1] > self::MAX_PIXELS) { + throw new InvalidUploadException('Image trop grande (au plus ' . self::MAX_SIDE . ' pixels de côté).'); + } + + $image = $mimeType === 'image/png' ? @imagecreatefrompng($source) : @imagecreatefromjpeg($source); + if ($image === false) { + throw new InvalidUploadException('Image illisible.'); + } + + if ($mimeType === 'image/jpeg') { + $image = $this->upright($image, $source); + } + + $saved = $mimeType === 'image/png' ? $this->savePng($image, $destination) : @imagejpeg($image, $destination, self::JPEG_QUALITY); + imagedestroy($image); + if (!$saved) { + throw new InvalidUploadException("Échec de l'enregistrement du fichier."); + } + } + + private function savePng(\GdImage $image, string $destination): bool + { + imagealphablending($image, false); + imagesavealpha($image, true); + + return @imagepng($image, $destination, 6); + } + + /** Applique l'orientation EXIF d'une photo (rotations 90/180/270 et miroirs simples) ; sans EXIF lisible, l'image est inchangée. */ + private function upright(\GdImage $image, string $source): \GdImage + { + if (!function_exists('exif_read_data')) { + return $image; + } + $exif = @exif_read_data($source); + $orientation = is_array($exif) ? (int) ($exif['Orientation'] ?? 1) : 1; + + $result = match ($orientation) { + 3 => imagerotate($image, 180, 0), + 6 => imagerotate($image, -90, 0), + 8 => imagerotate($image, 90, 0), + 2 => imageflip($image, IMG_FLIP_HORIZONTAL) ? $image : false, + 4 => imageflip($image, IMG_FLIP_VERTICAL) ? $image : false, + default => $image, + }; + + return $result === false ? $image : $result; + } +} diff --git a/tests/Group/Service/GroupDocumentServiceTest.php b/tests/Group/Service/GroupDocumentServiceTest.php index 87b8d167..9d2baf6b 100644 --- a/tests/Group/Service/GroupDocumentServiceTest.php +++ b/tests/Group/Service/GroupDocumentServiceTest.php @@ -40,12 +40,12 @@ protected function tearDown(): void parent::tearDown(); } - private function makeService(int $maxDocuments = 20): array + private function makeService(int $maxDocuments = 20, int $maxBytes = 100 * 1024 * 1024): array { $groupRepository = new MysqlGroupRepository($this->pdo); $userRepository = new MysqlUserRepository($this->pdo); $documentRepository = new MysqlGroupDocumentRepository($this->pdo); - $service = new GroupDocumentService($documentRepository, $groupRepository, new TransactionRunner($this->pdo), $this->storagePath, $maxDocuments); + $service = new GroupDocumentService($documentRepository, $groupRepository, new TransactionRunner($this->pdo), $this->storagePath, $maxDocuments, $maxBytes); return [$service, $groupRepository, $userRepository, $documentRepository]; } @@ -325,6 +325,11 @@ public function lockGroupQuota(int $groupId): void $this->inner->lockGroupQuota($groupId); } + public function totalBytesByGroup(int $groupId): int + { + return $this->inner->totalBytesByGroup($groupId); + } + public function save(GroupDocument $document): GroupDocument { throw new \PDOException('SQLSTATE : base indisponible'); @@ -406,4 +411,127 @@ public function testTheFilesOfAGroupCanBeListedThenRemovedWhenTheGroupIsDeleted( $service->remove(['/inexistant/fichier.pdf']); // best effort : un fichier déjà absent n'est jamais une erreur self::addToAssertionCount(1); } + + /** Un PNG de $width × $height fabriqué avec GD, plus d'éventuels octets parasites à la suite. */ + private function pngBytes(int $width = 4, int $height = 3, string $trailing = ''): string + { + $image = imagecreatetruecolor($width, $height); + ob_start(); + imagepng($image); + + return (string) ob_get_clean() . $trailing; + } + + /** Un JPEG de $width × $height fabriqué avec GD ; `$segments` : segments (marqueur + contenu) insérés juste après l'en-tête SOI. */ + private function jpegBytes(int $width = 4, int $height = 3, string $segments = ''): string + { + $image = imagecreatetruecolor($width, $height); + ob_start(); + imagejpeg($image); + $jpeg = (string) ob_get_clean(); + + return substr($jpeg, 0, 2) . $segments . substr($jpeg, 2); + } + + /** Segment de commentaire JPEG (FFFE) : un endroit où des données parasites ou personnelles se cachent. */ + private function jpegComment(string $text): string + { + return "\xFF\xFE" . pack('n', strlen($text) + 2) . $text; + } + + /** Segment EXIF (APP1) portant l'orientation $orientation (6 = à tourner de 90° dans le sens horaire). */ + private function exifOrientation(int $orientation): string + { + $tiff = "MM\x00\x2A\x00\x00\x00\x08" . pack('n', 1) . pack('nnN', 0x0112, 3, 1) . pack('n', $orientation) . "\x00\x00" . "\x00\x00\x00\x00"; + $body = "Exif\x00\x00" . $tiff; + + return "\xFF\xE1" . pack('n', strlen($body) + 2) . $body; + } + + private function uploadAsManager(GroupDocumentService $service, string $content, string $name): GroupDocument + { + [, $groupRepository, $userRepository] = $this->makeService(); + $group = $groupRepository->save(new Group(0, 'Groupe ' . uniqid(), null, null, 'contact@example.test')); + $manager = $this->createUser($userRepository, uniqid('m') . '@rehearsalbox.test'); + $groupRepository->addMember($group->id(), $manager->id(), GroupUserRole::Gestionnaire); + + return $service->upload($group->id(), $manager->id(), $this->fakeUploadedFile($content, $name), $name, strlen($content)); + } + + #[Test] + public function testAPictureIsReEncodedSoTrailingBytesAndHiddenSegmentsAreDropped(): void + { + [$service] = $this->makeService(); + + $png = $this->uploadAsManager($service, $this->pngBytes(4, 3, ''), 'p.png'); + $jpeg = $this->uploadAsManager($service, $this->jpegBytes(4, 3, $this->jpegComment('POSITION-GPS-SECRETE')), 'p.jpg'); + + foreach ([$png, $jpeg] as $document) { + $stored = (string) file_get_contents($service->pathOf($document)); + self::assertStringNotContainsString('