fix: prevent arbitrary file write via unsanitized SVG/MVG bitmap previews [10.16] (OC10-164)

changelog/unreleased/41828

@@ -0,0 +1,15 @@
+Security: Reject SVG/script content before it reaches ImageMagick bitmap previews
+
+Bitmap previews (PDF, Font, ...) sanitized SVG content before decoding it, but
+fell back to the original, unsanitized bytes whenever the sanitizer could not
+parse the input - which happened for any malformed SVG or non-XML payload,
+not only for genuinely broken SVG files. A crafted malformed SVG or a raw MVG
+script could therefore reach ImageMagick unsanitized and trigger an MSL
+script that reads or writes arbitrary files as the web server user.
+
+Bitmap previews no longer attempt to sanitize and fall back; they now reject
+any content that is detected as text, XML, SVG, or MVG before ImageMagick
+ever sees it, and decode through the same hardened Imagick options already
+used by the dedicated SVG preview provider.
+
+https://github.com/owncloud/core/pull/41828

lib/private/Preview/Bitmap.php

@@ -25,6 +25,7 @@
 namespace OC\Preview;
 
 use Imagick;
+use OC\Image\ImagickFactory;
 use OC\Preview;
 use OCP\Files\File;
 use OCP\Files\FileInfo;
@@ -85,18 +86,16 @@ public function isAvailable(FileInfo $file) {
 	 * @return Imagick
 	 */
 	private function getResizedPreview($stream, int $maxX, int $maxY): Imagick {
-		# file content can be SVG - we need to sanitize it first
 		$content = \stream_get_contents($stream);
-		$output = SVG::sanitizeSVGContent($content);
-		# in case the content is not an SVG we use the original content
-		if ($output === '') {
-			$output = $content;
+
+		if ($this->isDangerousToDecode($content)) {
+			throw new \RuntimeException('Refusing to decode text-based content for a bitmap preview');
 		}
 
-		$bp = new Imagick();
+		$bp = ImagickFactory::create();
 
 		# setIteratorIndex(0) will make previews to be generated from the first page
-		$bp->readImageBlob($output);
+		$bp->readImageBlob($content);
 		$bp->setIteratorIndex(0);
 
 		$bp = $this->resize($bp, $maxX, $maxY);
@@ -106,6 +105,23 @@ private function getResizedPreview($stream, int $maxX, int $maxY): Imagick {
 		return $bp;
 	}
 
+	/**
+	 * Bitmap providers must never hand text-based content (SVG, XML, or any other
+	 * text/* type, e.g. a raw MVG script) to Imagick::readImageBlob() - ImageMagick's
+	 * text/vector coders can be abused to read and write arbitrary files.
+	 */
+	private function isDangerousToDecode(string $content): bool {
+		$mimeType = \OC::$server->getMimeTypeDetector()->detectString($content);
+		$mimeType = \strtolower(\trim(\explode(';', $mimeType, 2)[0]));
+
+		// libmagic reports "image/svg" without the "+xml" suffix on some PHP/OS builds
+		if (\strpos($mimeType, 'text/') === 0 || \strpos($mimeType, 'image/svg') === 0) {
+			return true;
+		}
+
+		return \in_array($mimeType, ['application/xml', 'image/x-mvg'], true);
+	}
+
 	/**
 	 * Returns a resized \Imagick object
 	 *

lib/private/Preview/SVG.php

@@ -54,6 +54,9 @@ public function getThumbnail(File $file, $maxX, $maxY, $scalingUp) {
 
 			# sanitize SVG content
 			$output = self::sanitizeSVGContent($content);
+			if ($output === null) {
+				return false;
+			}
 
 			$imagick->readImageBlob($output);
 			$imagick->setImageFormat('png32');
@@ -81,7 +84,7 @@ public function isAvailable(FileInfo $file) {
 		return true;
 	}
 
-	public static function sanitizeSVGContent(string $content): string {
+	public static function sanitizeSVGContent(string $content): ?string {
 		$sanitizer = new DOMSanitizer(DOMSanitizer::SVG);
 		$sanitizer->addDisallowedTags(['image']);
 		$sanitizer->addDisallowedAttributes(['xlink:href']);
@@ -90,6 +93,10 @@ public static function sanitizeSVGContent(string $content): string {
 		// XML errors are expected here if the SVG is malformed
 		\libxml_clear_errors();
 
+		if (!\is_string($sanitized_content)) {
+			return null;
+		}
+
 		return $sanitized_content;
 	}
 }

tests/lib/Preview/SanitizeTest.php

@@ -44,10 +44,10 @@ public function test(string $svgContent, Bitmap $provider): void {
 		$file->method('getContent')->willReturn($svgContent);
 		$file->method('fopen')->willReturn($stream);
 
-		# create the preview
+		# create the preview - SVG/text/script-shaped content must never reach Imagick via a Bitmap provider
 		$return = $provider->getThumbnail($file, 32, 32, false);
 
-		$this->assertImage(__DIR__ . '/white-32x32.png', $return);
+		$this->assertFalse($return);
 	}
 
 	public function providesSVG(): Generator {
@@ -56,10 +56,35 @@ public function providesSVG(): Generator {
 <svg xmlns="http://www.w3.org/2000/svg" width="800" height="800">
 	<image href="$embeddedImagePath" width="400" height="400"></image>
 </svg>
+SVG;
+
+		# malformed SVG (unclosed <image>) - the DOM sanitizer cannot parse this and
+		# used to fall back to the raw, unsanitized content
+		$malformedSvgWithMslHref = <<<SVG
+<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink" width="10" height="10">
+	<image xlink:href="MSL:/tmp/oc10-164-payload.msl" width="10" height="10">
+</svg>
+SVG;
+
+		$rawMvg = <<<MVG
+push graphic-context
+viewbox 0 0 64 64
+fill 'url(msl:/tmp/oc10-164-payload.msl)'
+pop graphic-context
+MVG;
+
+		$wellFormedSvg = <<<SVG
+<svg xmlns="http://www.w3.org/2000/svg" width="10" height="10"><rect width="10" height="10" fill="green"/></svg>
 SVG;
 
 		# all Bitmap based providers use the same thumbnailing logic - two is enough ....
-		yield 'PDF provider' => [$svgContent0, new PDF()];
-		yield 'Font Provider' => [$svgContent0, new Font()];
+		yield 'PDF provider - image tag' => [$svgContent0, new PDF()];
+		yield 'Font Provider - image tag' => [$svgContent0, new Font()];
+		yield 'PDF provider - malformed SVG with MSL href' => [$malformedSvgWithMslHref, new PDF()];
+		yield 'Font Provider - malformed SVG with MSL href' => [$malformedSvgWithMslHref, new Font()];
+		yield 'PDF provider - raw MVG' => [$rawMvg, new PDF()];
+		yield 'Font Provider - raw MVG' => [$rawMvg, new Font()];
+		yield 'PDF provider - well-formed SVG' => [$wellFormedSvg, new PDF()];
+		yield 'Font Provider - well-formed SVG' => [$wellFormedSvg, new Font()];
 	}
 }