fix: pin the Imagick coder per provider instead of letting it sniff (OC10-164)

changelog/unreleased/41832

@@ -0,0 +1,16 @@
+Security: Pin the Imagick coder for each bitmap preview provider
+
+Bitmap previews decoded content with no format hint, so ImageMagick's own
+content-sniffing - independent of the mime-type check that decides whether a
+preview is even attempted - could still pick a different coder than the one
+a provider actually serves. PostScript-looking content, which the mime check
+must allow through for the PDF and Postscript providers, could therefore
+still reach the Ghostscript delegate through any other bitmap provider (SGI,
+Font, Illustrator, Photoshop, TIFF, Heic).
+
+Each provider now pins the exact Imagick coder it expects instead of letting
+ImageMagick guess from the file's content, using a tmpfs-backed temporary
+file (falling back to disk if none is available) so the pin adds no disk
+I/O to preview generation.
+
+https://github.com/owncloud/core/pull/41832

config/config.sample.php

@@ -1443,6 +1443,23 @@
  */
 'tempdirectory' => '/tmp/owncloudtemp',
 
+/**
+ * Define the location for RAM-backed (tmpfs) temporary files
+ * Used for a small number of short-lived, write-then-immediately-read
+ * scratch files (currently just bitmap/vector preview generation), to avoid
+ * unnecessary disk I/O for content that is read back within milliseconds of
+ * being written.
+ *
+ * Auto-detects /dev/shm by default and silently falls back to the regular
+ * 'tempdirectory' location above if no writable tmpfs mount is found, or if
+ * the temporary file cannot be created there - this is always a best-effort
+ * optimization, never a hard requirement.
+ *
+ * Set to a path to use a different tmpfs mount than /dev/shm. Set to false
+ * to disable RAM-backed temporary files entirely.
+ */
+'ramtempdirectory' => '/dev/shm',
+
 /**
  * Define the hashing cost
  * The hashing cost used by hashes generated by ownCloud.

lib/private/Preview/Bitmap.php

@@ -49,7 +49,7 @@ public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) {
 
 		// Creates \Imagick object from bitmap or vector file
 		try {
-			$bp = $this->getResizedPreview($stream, $maxX, $maxY);
+			$bp = $this->getResizedPreview($stream, $maxX, $maxY, $file->getMimeType());
 		} catch (\Exception $e) {
 			Util::writeLog('core', 'ImageMagick says: ' . $e->getmessage(), Util::ERROR);
 			return false;
@@ -82,10 +82,12 @@ public function isAvailable(FileInfo $file) {
 	 * @param resource $stream the handle of the file to convert
 	 * @param int $maxX
 	 * @param int $maxY
+	 * @param string $mimeType the file's own detected mime type, used to pin the
+	 *   Imagick coder so it can't be redirected by the file's actual content
 	 *
 	 * @return Imagick
 	 */
-	private function getResizedPreview($stream, int $maxX, int $maxY): Imagick {
+	private function getResizedPreview($stream, int $maxX, int $maxY, string $mimeType): Imagick {
 		$content = \stream_get_contents($stream);
 
 		if ($this->isDangerousToDecode($content)) {
@@ -94,8 +96,31 @@ private function getResizedPreview($stream, int $maxX, int $maxY): Imagick {
 
 		$bp = ImagickFactory::create();
 
+		# Pin the coder instead of letting Imagick's own content-sniffing pick one:
+		# reading with no format set re-derives the format from a ~130-entry magic
+		# table independently of isDangerousToDecode()'s check above, so content that
+		# looks like PostScript/PDF (which that check must allow through for the
+		# Postscript/PDF providers) would otherwise reach the Ghostscript delegate via
+		# any Bitmap provider, not just those two.
+		#
+		# The pin has to be a "FORMAT:path" read, not setFormat()+readImageBlob(): the
+		# latter does reliably reject a mismatched format, but for several coders here
+		# (PDF, EPS, AI, PSD, SGI, TIFF, HEIC, HEIF - i.e. all of them) it also silently
+		# skips the actual rasterization step, so setImageFormat('png') below ends up
+		# with no effect and getImageBlob() returns the original, undecoded bytes.
+		#
+		# Uses a RAM-backed (tmpfs) temp file rather than the regular disk-backed one:
+		# this write-then-immediately-read-then-delete file exists purely to give
+		# Imagick a path to pin against and never needs to survive on disk.
+		$tmpPath = \OC::$server->getTempManager()->getRamTemporaryFile();
+		\file_put_contents($tmpPath, $content);
+		try {
+			$bp->readImage($this->getImagickFormat($mimeType) . ':' . $tmpPath);
+		} finally {
+			\unlink($tmpPath);
+		}
+
 		# setIteratorIndex(0) will make previews to be generated from the first page
-		$bp->readImageBlob($content);
 		$bp->setIteratorIndex(0);
 
 		$bp = $this->resize($bp, $maxX, $maxY);
@@ -122,6 +147,12 @@ private function isDangerousToDecode(string $content): bool {
 		return \in_array($mimeType, ['application/xml', 'image/x-mvg'], true);
 	}
 
+	/**
+	 * Maps this provider's own detected mime type(s) to the Imagick coder name that
+	 * must decode them - the format pinned in getResizedPreview() above.
+	 */
+	abstract protected function getImagickFormat(string $mimeType): string;
+
 	/**
 	 * Returns a resized \Imagick object
 	 *

lib/private/Preview/Font.php

@@ -29,4 +29,13 @@ class Font extends Bitmap {
 	public function getMimeType() {
 		return '/application\/(?:font-sfnt|x-font$)/';
 	}
+
+	protected function getImagickFormat(string $mimeType): string {
+		if ($mimeType === 'application/x-font') {
+			return 'PFB';
+		}
+		# .otf and .ttf are indistinguishable by mime type alone (both application/font-sfnt);
+		# TTF is what actually decodes real font files here, both tagged variants included.
+		return 'TTF';
+	}
 }

lib/private/Preview/Heic.php

@@ -29,4 +29,11 @@ class Heic extends Bitmap {
 	public function getMimeType() {
 		return '/image\/hei(f|c)/';
 	}
+
+	protected function getImagickFormat(string $mimeType): string {
+		if ($mimeType === 'image/heif') {
+			return 'HEIF';
+		}
+		return 'HEIC';
+	}
 }

lib/private/Preview/Illustrator.php

@@ -30,4 +30,8 @@ class Illustrator extends Bitmap {
 	public function getMimeType() {
 		return '/application\/illustrator/';
 	}
+
+	protected function getImagickFormat(string $mimeType): string {
+		return 'AI';
+	}
 }

lib/private/Preview/Office.php

@@ -68,7 +68,10 @@ public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) {
 			$pdfPreview = $tmpDir . '/' . $pathInfo['filename'] . '.pdf';
 
 			# Note: no SVG sanitization of the file content required ....
-			$imagick = ImagickFactory::create($pdfPreview . '[0]');
+			# Pin the coder: this is LibreOffice's own PDF output, but readImageBlob()-style
+			# content-sniffing is avoided everywhere else Imagick decodes a file in this
+			# codebase, so pin it here too rather than rely on the ".pdf" path extension.
+			$imagick = ImagickFactory::create('PDF:' . $pdfPreview . '[0]');
 			$imagick->setImageFormat('jpg');
 		} catch (\Exception $e) {
 			@\unlink($pdfPreview);

lib/private/Preview/PDF.php

@@ -30,4 +30,8 @@ class PDF extends Bitmap {
 	public function getMimeType() {
 		return '/application\/pdf/';
 	}
+
+	protected function getImagickFormat(string $mimeType): string {
+		return 'PDF';
+	}
 }

lib/private/Preview/Photoshop.php

@@ -30,4 +30,8 @@ class Photoshop extends Bitmap {
 	public function getMimeType() {
 		return '/application\/x-photoshop/';
 	}
+
+	protected function getImagickFormat(string $mimeType): string {
+		return 'PSD';
+	}
 }

lib/private/Preview/Postscript.php

@@ -30,4 +30,8 @@ class Postscript extends Bitmap {
 	public function getMimeType() {
 		return '/application\/postscript/';
 	}
+
+	protected function getImagickFormat(string $mimeType): string {
+		return 'EPS';
+	}
 }

lib/private/Preview/SGI.php

@@ -27,4 +27,8 @@ class SGI extends Bitmap {
 	public function getMimeType() {
 		return '/image\/sgi/';
 	}
+
+	protected function getImagickFormat(string $mimeType): string {
+		return 'SGI';
+	}
 }

lib/private/Preview/SVG.php

@@ -58,7 +58,22 @@ public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) {
 				return false;
 			}
 
-			$imagick->readImageBlob($output);
+			# Pin the coder: reading with no format set would let Imagick's own content-
+			# sniffing pick the coder independently of the svg:sanitize/embed/decode
+			# options above and of the isDangerousToDecode()-style reasoning in Bitmap.php.
+			# This has to be a "SVG:path" read, not setFormat()+readImageBlob(): the latter
+			# silently skips the actual rasterization step, same as in Bitmap.php.
+			#
+			# RAM-backed like Bitmap.php's pin file: it is written, read and deleted again
+			# within this one call and exists purely to give Imagick a path to pin against,
+			# so it never needs to survive on disk.
+			$tmpPath = \OC::$server->getTempManager()->getRamTemporaryFile();
+			\file_put_contents($tmpPath, $output);
+			try {
+				$imagick->readImage('SVG:' . $tmpPath);
+			} finally {
+				\unlink($tmpPath);
+			}
 			$imagick->setImageFormat('png32');
 		} catch (\Exception $e) {
 			\OCP\Util::writeLog('core', $e->getmessage(), \OCP\Util::ERROR);

lib/private/Preview/TIFF.php

@@ -30,4 +30,8 @@ class TIFF extends Bitmap {
 	public function getMimeType() {
 		return '/image\/tiff/';
 	}
+
+	protected function getImagickFormat(string $mimeType): string {
+		return 'TIFF';
+	}
 }

lib/private/TempManager.php

@@ -40,6 +40,9 @@ class TempManager implements ITempManager {
 	protected array $current = [];
 	/** @var ?string i.e. /tmp on linux systems */
 	protected ?string $tmpBaseDir = null;
+	/** @var ?string tmpfs-backed directory to use for getRamTemporaryFile(), resolved lazily */
+	protected ?string $ramBaseDir = null;
+	protected bool $ramBaseDirChecked = false;
 	protected ILogger $logger;
 	protected IConfig $config;
 
@@ -144,6 +147,68 @@ public function getTemporaryFolder($postFix = '') {
 		return false;
 	}
 
+	/**
+	 * Create a temporary file backed by a tmpfs/RAM-backed mount when one is
+	 * available and writable, falling back transparently to the regular
+	 * disk-backed temporary directory otherwise. Intended for callers that
+	 * write-then-immediately-read short-lived scratch content and want to
+	 * avoid real disk I/O for it.
+	 *
+	 * Same failure contract as getTemporaryFile(): returns false only if
+	 * even the disk-backed fallback fails.
+	 */
+	public function getRamTemporaryFile(string $postFix = ''): string|false {
+		$ramBaseDir = $this->resolveRamBaseDir();
+		if ($ramBaseDir !== null) {
+			$file = @\tempnam($ramBaseDir, self::TMP_PREFIX);
+			if ($file !== false) {
+				$this->current[] = $file;
+
+				if ($postFix !== '') {
+					$fileNameWithPostfix = $this->buildFileNameWithSuffix($file, $postFix);
+					$old_umask = \umask(0077);
+					\touch($fileNameWithPostfix);
+					\umask($old_umask);
+					$this->current[] = $fileNameWithPostfix;
+					return $fileNameWithPostfix;
+				}
+
+				return $file;
+			}
+
+			$this->logger->debug(
+				'Could not create a RAM-backed temporary file in {dir}, falling back to disk',
+				['dir' => $ramBaseDir]
+			);
+		}
+
+		return $this->getTemporaryFile($postFix);
+	}
+
+	/**
+	 * Resolve and cache the tmpfs-backed directory to use for
+	 * getRamTemporaryFile(), or null if none is available/enabled.
+	 *
+	 * @return ?string
+	 */
+	private function resolveRamBaseDir(): ?string {
+		if ($this->ramBaseDirChecked) {
+			return $this->ramBaseDir;
+		}
+		$this->ramBaseDirChecked = true;
+
+		$configured = $this->config->getSystemValue('ramtempdirectory', null);
+		if ($configured === false) {
+			return $this->ramBaseDir = null;
+		}
+		$candidate = \is_string($configured) && $configured !== '' ? $configured : '/dev/shm';
+
+		if (\is_dir($candidate) && \is_writable($candidate)) {
+			return $this->ramBaseDir = $candidate;
+		}
+		return $this->ramBaseDir = null;
+	}
+
 	/**
 	 * Remove the temporary files and folders generated during this request
 	 */

lib/public/ITempManager.php

@@ -67,4 +67,15 @@ public function cleanOld();
 	 * @since 8.2.0
 	 */
 	public function getTempBaseDir();
+
+	/**
+	 * Create a temporary file backed by a tmpfs/RAM-backed mount when one is
+	 * available and writable, falling back transparently to the regular
+	 * disk-backed temporary directory otherwise. Intended for callers that
+	 * write-then-immediately-read short-lived scratch content and want to
+	 * avoid real disk I/O for it.
+	 *
+	 * @since 11.0.1
+	 */
+	public function getRamTemporaryFile(string $postFix = ''): string|false;
 }