fix(preview): check mime type before processing with Imagick

Signed-off-by: Varun Patil <varunpatil@ucla.edu>
nextcloud · Apr 8, 2024 · 5d4d84b · 5d4d84b
1 parent 1fb5486
commit 5d4d84b
Show file tree

Hide file tree

Showing 12 changed files with 111 additions and 13 deletions.
diff --git a/lib/private/Preview/Bitmap.php b/lib/private/Preview/Bitmap.php
@@ -37,6 +37,20 @@
  * @package OC\Preview
  */
 abstract class Bitmap extends ProviderV2 {
+	/**
+	 * List of MIME types that this preview provider is allowed to process.
+	 *
+	 * These should correspond to the MIME types *identified* by Imagemagick
+	 * for files to be processed by this provider. These do / will not
+	 * necessarily need to match the MIME types stored in the database
+	 * (which are identified by IMimeTypeDetector).
+	 *
+	 * @return string Regular expression
+	 */
+	protected function getAllowedMimeTypes(): string {
+		return '/invalid\/invalid/';
+	}
+
 	/**
 	 * {@inheritDoc}
 	 */
@@ -86,10 +100,19 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 	 * @param int $maxY
 	 *
 	 * @return \Imagick
+	 *
+	 * @throws \Exception
 	 */
 	private function getResizedPreview($tmpPath, $maxX, $maxY) {
 		$bp = new Imagick();
 
+		// Validate mime type
+		$bp->pingImage($tmpPath . '[0]');
+		$mimeType = $bp->getImageMimeType();
+		if (!preg_match($this->getAllowedMimeTypes(), $mimeType)) {
+			throw new \Exception('File mime type does not match the preview provider: ' . $mimeType);
+		}
+
 		// Layer 0 contains either the bitmap or a flat representation of all vector layers
 		$bp->readImage($tmpPath . '[0]');
 

diff --git a/lib/private/Preview/Font.php b/lib/private/Preview/Font.php
@@ -30,4 +30,11 @@ class Font extends Bitmap {
 	public function getMimeType(): string {
 		return '/application\/(?:font-sfnt|x-font$)/';
 	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function getAllowedMimeTypes(): string {
+		return '/(application|image)\/(?:font-sfnt|x-font|x-otf|x-ttf|x-pfb$)/';
+	}
 }
diff --git a/lib/private/Preview/HEIC.php b/lib/private/Preview/HEIC.php
@@ -44,7 +44,7 @@ class HEIC extends ProviderV2 {
 	 * {@inheritDoc}
 	 */
 	public function getMimeType(): string {
-		return '/image\/hei(f|c)/';
+		return '/image\/(x-)?hei(f|c)/';
 	}
 
 	/**
@@ -108,10 +108,20 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 	 * @param int $maxY
 	 *
 	 * @return \Imagick
+	 *
+	 * @throws \Exception
 	 */
 	private function getResizedPreview($tmpPath, $maxX, $maxY) {
 		$bp = new \Imagick();
 
+		// Some HEIC files just contain (or at least are identified as) other formats
+		// like JPEG. We just need to check if the image is safe to process.
+		$bp->pingImage($tmpPath . '[0]');
+		$mimeType = $bp->getImageMimeType();
+		if (!preg_match('/^image\/(x-)?(png|jpeg|gif|bmp|tiff|webp|hei(f|c)|avif)$/', $mimeType)) {
+			throw new \Exception('File mime type does not match the preview provider: ' . $mimeType);
+		}
+
 		// Layer 0 contains either the bitmap or a flat representation of all vector layers
 		$bp->readImage($tmpPath . '[0]');
 

diff --git a/lib/private/Preview/Illustrator.php b/lib/private/Preview/Illustrator.php
@@ -31,4 +31,11 @@ class Illustrator extends Bitmap {
 	public function getMimeType(): string {
 		return '/application\/illustrator/';
 	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function getAllowedMimeTypes(): string {
+		return '/application\/(illustrator|pdf)/';
+	}
 }
diff --git a/lib/private/Preview/PDF.php b/lib/private/Preview/PDF.php
@@ -31,4 +31,11 @@ class PDF extends Bitmap {
 	public function getMimeType(): string {
 		return '/application\/pdf/';
 	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function getAllowedMimeTypes(): string {
+		return '/application\/pdf/';
+	}
 }
diff --git a/lib/private/Preview/Photoshop.php b/lib/private/Preview/Photoshop.php
@@ -31,4 +31,11 @@ class Photoshop extends Bitmap {
 	public function getMimeType(): string {
 		return '/application\/x-photoshop/';
 	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function getAllowedMimeTypes(): string {
+		return '/(application|image)\/(x-photoshop|x-psd)/';
+	}
 }
diff --git a/lib/private/Preview/Postscript.php b/lib/private/Preview/Postscript.php
@@ -31,4 +31,11 @@ class Postscript extends Bitmap {
 	public function getMimeType(): string {
 		return '/application\/postscript/';
 	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function getAllowedMimeTypes(): string {
+		return '/application\/postscript/';
+	}
 }
diff --git a/lib/private/Preview/SGI.php b/lib/private/Preview/SGI.php
@@ -28,6 +28,13 @@ class SGI extends Bitmap {
 	 * {@inheritDoc}
 	 */
 	public function getMimeType(): string {
-		return '/image\/sgi/';
+		return '/image\/(x-)?sgi/';
+	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function getAllowedMimeTypes(): string {
+		return '/image\/(x-)?sgi/';
 	}
 }
diff --git a/lib/private/Preview/SVG.php b/lib/private/Preview/SVG.php
@@ -44,9 +44,6 @@ public function getMimeType(): string {
 	 */
 	public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		try {
-			$svg = new \Imagick();
-			$svg->setBackgroundColor(new \ImagickPixel('transparent'));
-
 			$content = stream_get_contents($file->fopen('r'));
 			if (substr($content, 0, 5) !== '<?xml') {
 				$content = '<?xml version="1.0" encoding="UTF-8" standalone="no"?>' . $content;
@@ -57,13 +54,25 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 				return null;
 			}
 
+			$svg = new \Imagick();
+
+			$svg->pingImageBlob($content);
+			$mimeType = $svg->getImageMimeType();
+			if (!preg_match($this->getMimeType(), $mimeType)) {
+				throw new \Exception('File mime type does not match the preview provider: ' . $mimeType);
+			}
+
+			$svg->setBackgroundColor(new \ImagickPixel('transparent'));
 			$svg->readImageBlob($content);
 			$svg->setImageFormat('png32');
 		} catch (\Exception $e) {
-			\OC::$server->get(LoggerInterface::class)->error($e->getMessage(), [
-				'exception' => $e,
-				'app' => 'core',
-			]);
+			\OC::$server->get(LoggerInterface::class)->error(
+				'File: ' . $file->getPath() . ' Imagick says:',
+				[
+					'exception' => $e,
+					'app' => 'core',
+				]
+			);
 			return null;
 		}
 

diff --git a/lib/private/Preview/TGA.php b/lib/private/Preview/TGA.php
@@ -28,6 +28,13 @@ class TGA extends Bitmap {
 	 * {@inheritDoc}
 	 */
 	public function getMimeType(): string {
-		return '/image\/t(ar)?ga/';
+		return '/image\/(x-)?t(ar)?ga/';
+	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function getAllowedMimeTypes(): string {
+		return '/image\/(x-)?t(ar)?ga/';
 	}
 }
diff --git a/lib/private/Preview/TIFF.php b/lib/private/Preview/TIFF.php
@@ -31,4 +31,11 @@ class TIFF extends Bitmap {
 	public function getMimeType(): string {
 		return '/image\/tiff/';
 	}
+
+	/**
+	 * {@inheritDoc}
+	 */
+	protected function getAllowedMimeTypes(): string {
+		return '/image\/tiff/';
+	}
 }
diff --git a/lib/private/PreviewManager.php b/lib/private/PreviewManager.php
@@ -376,9 +376,9 @@ protected function registerCoreProviders() {
 				'PSD' => ['mimetype' => '/application\/x-photoshop/', 'class' => Preview\Photoshop::class],
 				'EPS' => ['mimetype' => '/application\/postscript/', 'class' => Preview\Postscript::class],
 				'TTF' => ['mimetype' => '/application\/(?:font-sfnt|x-font$)/', 'class' => Preview\Font::class],
-				'HEIC' => ['mimetype' => '/image\/hei(f|c)/', 'class' => Preview\HEIC::class],
-				'TGA' => ['mimetype' => '/image\/t(ar)?ga/', 'class' => Preview\TGA::class],
-				'SGI' => ['mimetype' => '/image\/sgi/', 'class' => Preview\SGI::class],
+				'HEIC' => ['mimetype' => '/image\/(x-)?hei(f|c)/', 'class' => Preview\HEIC::class],
+				'TGA' => ['mimetype' => '/image\/(x-)?t(ar)?ga/', 'class' => Preview\TGA::class],
+				'SGI' => ['mimetype' => '/image\/(x-)?sgi/', 'class' => Preview\SGI::class],
 			];
 
 			foreach ($imagickProviders as $queryFormat => $provider) {