#9253 Check image validity when creating announcement

pkp · Nov 8, 2023 · f999bb8 · f999bb8
1 parent dd4c345
commit f999bb8
Show file tree

Hide file tree

Showing 3 changed files with 31 additions and 16 deletions.
diff --git a/api/v1/announcements/PKPAnnouncementHandler.php b/api/v1/announcements/PKPAnnouncementHandler.php
@@ -224,10 +224,13 @@ public function add($slimRequest, $response, $args)
         try {
             $announcementId = Repo::announcement()->add($announcement);
         } catch (StoreTemporaryFileException $e) {
-            $announcement = Repo::announcement()->get($announcementId);
-            Repo::announcement()->delete($announcement);
+            $announcementId = $e->dataObject->getId();
+            if ($announcementId) {
+                $announcement = Repo::announcement()->get($announcementId);
+                Repo::announcement()->delete($announcement);
+            }
             return $response->withStatus(400)->withJson([
-                'image' => __('api.400.errorUploadingImage')
+                'image' => [__('api.400.errorUploadingImage')]
             ]);
         }
 

diff --git a/classes/announcement/Repository.php b/classes/announcement/Repository.php
@@ -19,11 +19,13 @@
 use PKP\context\Context;
 use PKP\core\Core;
 use PKP\core\exceptions\StoreTemporaryFileException;
+use PKP\core\PKPString;
 use PKP\file\FileManager;
 use PKP\file\TemporaryFile;
 use PKP\file\TemporaryFileManager;
 use PKP\plugins\Hook;
 use PKP\services\PKPSchemaService;
+use PKP\user\User;
 use PKP\validation\ValidatorFactory;
 
 class Repository
@@ -230,6 +232,9 @@ protected function handleImageUpload(Announcement $announcement): void
             $temporaryFileManager = new TemporaryFileManager();
             $temporaryFile = $temporaryFileManager->getFile((int) $image['temporaryFileId'], $user?->getId());
             $filePath = $this->getImageSubdirectory() . '/' . $this->getImageFilename($announcement, $temporaryFile);
+            if (!$this->isValidImage($temporaryFile, $filePath, $user, $announcement)) {
+                throw new StoreTemporaryFileException($temporaryFile, $filePath, $user, $announcement);
+            }
             if ($this->storeTemporaryFile($temporaryFile, $filePath, $user->getId(), $announcement)) {
                 $announcement->setImage(
                     $this->getImageData($announcement, $temporaryFile)
@@ -324,4 +329,24 @@ protected function deleteImage(Announcement $announcement): void
             );
         }
     }
+
+    /**
+     * Check that temporary file is an image
+     */
+    protected function isValidImage(TemporaryFile $temporaryFile): bool
+    {
+        if (getimagesize($temporaryFile->getFilePath()) === false) {
+            return false;
+        }
+        $extension = pathinfo($temporaryFile->getOriginalFileName(), PATHINFO_EXTENSION);
+        $fileManager = new FileManager();
+        $extensionFromMimeType = $fileManager->getImageExtension(
+            PKPString::mime_content_type($temporaryFile->getFilePath())
+        );
+        if ($extensionFromMimeType !== '.' . $extension) {
+            return false;
+        }
+
+        return true;
+    }
 }
diff --git a/classes/file/PKPPublicFileManager.php b/classes/file/PKPPublicFileManager.php
@@ -93,19 +93,6 @@ public function copySiteFile($sourceFile, $destFileName)
         return $this->copyFile($sourceFile, $this->getSiteFilesPath() . '/' . $destFileName);
     }
 
-    /**
-     * Copy a file to a context's public directory.
-     *
-     * @param string $sourceFile the source of the file to copy
-     * @param string $destFileName the destination file name
-     *
-     * @return bool
-     */
-    public function copySiteFile($sourceFile, $destFileName)
-    {
-        return $this->copyFile($sourceFile, $this->getSiteFilesPath() . '/' . $destFileName);
-    }
-
     /**
      * Copy a file to a site's public directory.
      *