Merge pull request #62273 from nextcloud/backport/62202/stable23

[stable23] fix(preview): properly handle encoded content

Author: nickvergessen

lib/private/Preview/SVG.php

@@ -48,15 +48,19 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 			$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;
+			if ($content === false) {
+				return null;
 			}
-
-			// Do not parse SVG files with references
-			if (stripos($content, 'xlink:href') !== false) {
+			// check if the file can be processed by this provider
+			if (!$this->canBeProcessed($content)) {
 				return null;
 			}
 
+			$content = ltrim($content);
+			if (substr($content, 0, 5) !== '<?xml') {
+				$content = '<?xml version="1.0" encoding="UTF-8" standalone="no"?>' . $content;
+			}
+
 			$svg->readImageBlob($content);
 			$svg->setImageFormat('png32');
 		} catch (\Exception $e) {
@@ -78,4 +82,30 @@ public function getThumbnail(File $file, int $maxX, int $maxY): ?IImage {
 		}
 		return null;
 	}
+
+	/**
+	 * Check if the file can be processed by this provider,
+	 * meaning the SVG is safe to be processed and does not contain any external references.
+	 */
+	protected function canBeProcessed(string $content): bool {
+		// check for allowed encodings and convert if necessary
+		$encoding = mb_detect_encoding($content, ['ISO-2022-JP', 'UTF-8', 'ISO-8859-1'], true);
+		if ($encoding === false) {
+			return false;
+		} elseif ($encoding !== 'UTF-8') {
+			$content = mb_convert_encoding($content, 'UTF-8', $encoding);
+		}
+
+		// Strip all non-printable/control characters except newlines/tabs
+		$content = preg_replace('/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/', '', $content);
+		if ($content === null) {
+			return false;
+		}
+
+		// check for any potential external reference (include custom namespace prefix)
+		if (preg_match('/["\s\']([a-z_][a-z0-9_.-]*:)?href\s*=/i', $content)) {
+			return false;
+		}
+		return true;
+	}
 }

tests/lib/Preview/Provider.php

@@ -22,6 +22,9 @@
 namespace Test\Preview;
 
 use OC\Files\Node\File;
+use OC\Preview\Provider as PreviewProvider;
+use OCP\Files\IRootFolder;
+use OCP\Preview\IProviderV2;
 
 abstract class Provider extends \Test\TestCase {
 
@@ -31,7 +34,7 @@ abstract class Provider extends \Test\TestCase {
 	protected $width;
 	/** @var int */
 	protected $height;
-	/** @var \OC\Preview\Provider */
+	/** @var IProviderV2|PreviewProvider */
 	protected $provider;
 	/** @var int */
 	protected $maxWidth = 1024;
@@ -138,8 +141,10 @@ protected function prepareTestFile($fileName, $fileContent) {
 	 * @return bool|\OCP\IImage
 	 */
 	private function getPreview($provider) {
-		$file = new File(\OC::$server->getRootFolder(), $this->rootView, $this->imgPath);
-		$preview = $provider->getThumbnail($file, $this->maxWidth, $this->maxHeight, $this->scalingUp);
+		$file = new File(\OC::$server->get(IRootFolder::class), $this->rootView, $this->imgPath);
+		$preview = ($provider instanceof PreviewProvider)
+			? $provider->getThumbnail($file, $this->maxWidth, $this->maxHeight, $this->scalingUp, $this->rootView)
+			: $provider->getThumbnail($file, $this->maxWidth, $this->maxHeight, $this->scalingUp);
 
 		if (get_class($this) === BitmapTest::class && $preview === null) {
 			$this->markTestSkipped('An error occured while operating with Imagick.');

tests/lib/Preview/SVGTest.php

@@ -21,6 +21,8 @@
 
 namespace Test\Preview;
 
+use OCP\Files\File;
+
 /**
  * Class SVGTest
  *
@@ -43,4 +45,83 @@ protected function setUp(): void {
 			$this->markTestSkipped('No SVG provider present');
 		}
 	}
+
+	public function dataGetThumbnailSVGHref(): array {
+		return [
+			['href'],
+			[' href'],
+			["\nhref"],
+			['xlink:href'],
+			[' xlink:href'],
+			["\nxlink:href"],
+		];
+	}
+
+	/**
+	 * @dataProvider dataGetThumbnailSVGHref
+	 * @requires extension imagick
+	 */
+	public function testGetThumbnailSVGHref(string $content): void {
+		$handle = fopen('php://temp', 'w+');
+		fwrite($handle, '<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg">
+  <image x="0" y="0"' . $content . '="fxlogo.png" height="100" width="100" />
+</svg>');
+		rewind($handle);
+
+		$file = $this->createMock(File::class);
+		$file->method('fopen')
+			->willReturn($handle);
+
+		self::assertNull($this->provider->getThumbnail($file, 512, 512));
+	}
+
+	/**
+	 * @dataProvider dataGetThumbnailSVGHrefNamespace
+	 * @requires extension imagick
+	 */
+	public function testGetThumbnailSvgHrefNamespace(string $namespace): void {
+		$handle = fopen('php://temp', 'w+');
+		fwrite($handle, '<svg viewBox="0 0 100 100" xmlns="http://www.w3.org/2000/svg" xmlns:' . $namespace . '="http://www.w3.org/1999/xlink">
+  <image x="0" y="0" ' . $namespace . ':href="fxlogo.png" height="100" width="100" />
+</svg>');
+		rewind($handle);
+
+		$file = $this->createMock(File::class);
+		$file->method('fopen')
+			->willReturn($handle);
+
+		self::assertNull($this->provider->getThumbnail($file, 512, 512));
+	}
+
+	public static function dataGetThumbnailSVGHrefNamespace(): array {
+		return [
+			['xlink'],
+			['foo'],
+			['_foo'],
+			['fo_12'],
+			['foo-bar'],
+			['Fo_B1-ar'],
+		];
+	}
+
+	/**
+	 * @dataProvider dataGetThumbnailSvgEncoded
+	 * @requires extension imagick
+	 */
+	public function testGetThumbnailSvgEncoded(string $content): void {
+		$handle = fopen('php://temp', 'w+');
+		fwrite($handle, $content);
+		rewind($handle);
+
+		$file = $this->createMock(File::class);
+		$file->method('fopen')
+			->willReturn($handle);
+		self::assertNull($this->provider->getThumbnail($file, 512, 512));
+	}
+
+	public static function dataGetThumbnailSvgEncoded(): array {
+		return [
+			'iso-2022-jp' => ["<?xml version=\"1.0\" encoding=\"ISO-2022-JP\"?>\n<svg width=\"700\" height=\"700\" xmlns=\"http://www.w3.org/2000/svg\">\n<i\x1b(Bmage width=\"700\" height=\"700\" h\x1b(Bref=\"text:/proc/cpuinfo\" />\n</svg>"],
+		];
+	}
 }