fix(message): render received external SVG images

lib/Controller/ProxyController.php

@@ -10,9 +10,11 @@
 
 namespace OCA\Mail\Controller;
 
+use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\Html\ProxyHmacGenerator;
 use OCA\Mail\Http\ProxyDownloadResponse;
 use OCA\Mail\Service\MailManager;
+use OCA\Mail\Service\SvgSanitizer;
 use OCP\AppFramework\Controller;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Http;
@@ -28,6 +30,7 @@
 use Psr\Log\LoggerInterface;
 use function file_get_contents;
 use function hash_equals;
+use function is_string;
 
 #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
 class ProxyController extends Controller {
@@ -44,6 +47,7 @@ public function __construct(
 		private ProxyHmacGenerator $hmacGenerator,
 		private LoggerInterface $logger,
 		private MailManager $mailManager,
+		private SvgSanitizer $svgSanitizer,
 		private ?string $userId,
 	) {
 		parent::__construct($appName, $request);
@@ -65,6 +69,7 @@ public function __construct(
 	 *       mail does not know whether the mail has been opened.
 	 *
 	 * @return Response|ProxyDownloadResponse
+	 * @throws ServiceException
 	 */
 	#[UserRateLimit(limit: 50, period: 60)]
 	public function proxy(string $src, ?int $id, ?string $hmac): Response {
@@ -73,7 +78,7 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 
 		// If strict cookies are set it means we come from the same domain so no open redirect
 		if (!$this->request->passesStrictCookieCheck()) {
-			$content = file_get_contents(__DIR__ . '/../../img/blocked-image.png');
+			$content = $this->getBlockedImage();
 			return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
 		}
 
@@ -95,17 +100,45 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 		try {
 			$response = $client->get($src);
 			$content = $response->getBody();
+			// Not a stream request, so the body is a string, never a resource.
+			assert(is_string($content));
 		} catch (ClientExceptionInterface $e) {
 			$this->logger->notice('Unable to proxy image', ['exception' => $e]);
-			$content = file_get_contents(__DIR__ . '/../../img/blocked-image.png');
+			$content = $this->getBlockedImage();
 		} catch (LocalServerException $e) {
 			$this->logger->warning('Prevented image proxy access to forbidden URL', [
 				'blockedUrl' => $src,
 				'exception' => $e,
 			]);
-			$content = file_get_contents(__DIR__ . '/../../img/blocked-image.png');
+			$content = $this->getBlockedImage();
+		}
+
+		// Browsers sniff raster image formats in <img> tags, but they refuse to
+		// render SVG unless it is served with the image/svg+xml content type.
+		// Detect and sanitise SVG markup so external SVG logos are displayed
+		// instead of staying blank. Sanitising also strips any active content in
+		// case the response is fetched through a direct (non-<img>) navigation.
+		if ($this->svgSanitizer->looksLikeSvg($content)) {
+			$sanitized = $this->svgSanitizer->sanitize($content);
+			if ($sanitized === '') {
+				$content = $this->getBlockedImage();
+				return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
+			}
+			return new ProxyDownloadResponse($sanitized, $src, 'image/svg+xml');
 		}
 
 		return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
 	}
+
+	/**
+	 * @throws ServiceException
+	 */
+	private function getBlockedImage(): string {
+		$content = file_get_contents(__DIR__ . '/../../img/blocked-image.png');
+		if ($content === false) {
+			throw new ServiceException('Could not read blocked image');
+		}
+
+		return $content;
+	}
 }

lib/Service/SvgSanitizer.php

@@ -0,0 +1,216 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Mail\Service;
+
+use DOMAttr;
+use DOMDocument;
+use DOMElement;
+use DOMXPath;
+
+/**
+ * Removes active content from SVG markup before it is embedded into or sent
+ * with a message. SVGs are rendered in an <img>/CID context where scripts do
+ * not execute, but they are still sanitised as defence in depth: any document
+ * that cannot be parsed safely is dropped entirely.
+ */
+class SvgSanitizer {
+	/** Elements that can carry or execute active content. */
+	private const FORBIDDEN_ELEMENTS = [
+		'script',
+		'foreignObject',
+		'handler',
+		'listener',
+		'set',
+	];
+
+	/**
+	 * Local names of attributes that carry URL references and must not point
+	 * off-document. Matched without their namespace prefix because a reference
+	 * can be bound to any prefix, not just the conventional xlink one.
+	 */
+	private const URL_ATTRIBUTES = ['href', 'src', 'action', 'formaction'];
+
+	/** Encodings accepted for inspection; anything else is rejected outright. */
+	private const ALLOWED_ENCODINGS = ['UTF-8', 'ISO-2022-JP', 'ISO-8859-1'];
+
+	/** Reject payloads larger than this to prevent DoS via oversized documents. */
+	private const MAX_SVG_BYTES = 2 * 1024 * 1024;
+
+	/**
+	 * @param string $svg The raw (decoded) SVG markup
+	 * @return string The sanitised markup, or an empty string if it cannot be
+	 *                parsed safely
+	 */
+	public function sanitize(string $svg): string {
+		if (trim($svg) === '' || strlen($svg) > self::MAX_SVG_BYTES) {
+			return '';
+		}
+
+		// The checks below compare plain strings, so they run on a normalised
+		// copy of the payload: an exotic encoding or an embedded control
+		// character would otherwise hide markup that the XML parser still sees.
+		$inspectable = $this->normalize($svg);
+		if ($inspectable === null) {
+			return '';
+		}
+
+		// A DOCTYPE or entity declaration is not needed for plain SVG graphics
+		// and is a common XXE / entity-expansion vector. Reject such documents.
+		if (preg_match('/<!DOCTYPE|<!ENTITY/i', $inspectable) === 1) {
+			return '';
+		}
+
+		// An XSL/Transform namespace signals a client-side transformation
+		// stylesheet that can execute JavaScript in some browsers. Reject the
+		// document outright, matching server-side hardening in nextcloud/server.
+		if (str_contains($inspectable, 'http://www.w3.org/1999/XSL/Transform')) {
+			return '';
+		}
+
+		$dom = new DOMDocument();
+		$previousErrors = libxml_use_internal_errors(true);
+		// LIBXML_NONET forbids any network access while parsing.
+		$loaded = $dom->loadXML($svg, LIBXML_NONET);
+		libxml_clear_errors();
+		libxml_use_internal_errors($previousErrors);
+
+		if (!$loaded || $dom->documentElement === null) {
+			return '';
+		}
+
+		$xpath = new DOMXPath($dom);
+
+		// Remove processing instructions, e.g. an xml-stylesheet PI pointing at an
+		// XSL sheet. Document-level PIs are already excluded by
+		// saveXML($dom->documentElement), but PIs nested inside the root element
+		// are handled here.
+		$pis = $xpath->query('//processing-instruction()');
+		if ($pis !== false) {
+			foreach (iterator_to_array($pis) as $pi) {
+				$pi->parentNode?->removeChild($pi);
+			}
+		}
+
+		// Remove dangerous elements. Matching on the local name catches them
+		// regardless of any namespace prefix (e.g. <x:script>).
+		foreach (self::FORBIDDEN_ELEMENTS as $tag) {
+			$nodes = $xpath->query('//*[local-name() = "' . $tag . '"]');
+			if ($nodes !== false) {
+				foreach (iterator_to_array($nodes) as $node) {
+					$node->parentNode?->removeChild($node);
+				}
+			}
+		}
+
+		// Sanitise <style> element content: strip external CSS url() references.
+		$styleNodes = $xpath->query('//*[local-name() = "style"]');
+		if ($styleNodes !== false) {
+			foreach ($styleNodes as $node) {
+				$node->textContent = $this->stripCssUrls($node->textContent);
+			}
+		}
+
+		$elements = $xpath->query('//*');
+		if ($elements !== false) {
+			foreach ($elements as $element) {
+				if ($element instanceof DOMElement) {
+					$this->stripDangerousAttributes($element);
+				}
+			}
+		}
+
+		$result = $dom->saveXML($dom->documentElement);
+		return $result === false ? '' : $result;
+	}
+
+	/**
+	 * Heuristically decide whether the given bytes are an SVG document.
+	 */
+	public function looksLikeSvg(string $content): bool {
+		$start = ltrim($content);
+		if (str_starts_with($start, "\xEF\xBB\xBF")) {
+			$start = ltrim(substr($start, 3));
+		}
+		$hasSvgPrologue = str_starts_with($start, '<?xml')
+			|| stripos($start, '<svg') === 0;
+		return $hasSvgPrologue && stripos($content, '<svg') !== false;
+	}
+
+	/**
+	 * Normalise the payload for the textual checks: decode it to UTF-8 and drop
+	 * the control characters that a plain string comparison would trip over but
+	 * the XML parser ignores. Mirrors the hardening of nextcloud/server#62162.
+	 *
+	 * @return string|null Null if the payload is not in an accepted encoding
+	 */
+	private function normalize(string $svg): ?string {
+		$encoding = mb_detect_encoding($svg, self::ALLOWED_ENCODINGS, true);
+		if ($encoding === false) {
+			return null;
+		}
+		if ($encoding !== 'UTF-8') {
+			$converted = mb_convert_encoding($svg, 'UTF-8', $encoding);
+			if (!is_string($converted)) {
+				return null;
+			}
+			$svg = $converted;
+		}
+
+		// Strip non-printable characters, but keep tab, newline and carriage
+		// return as those are legal XML whitespace.
+		return preg_replace('/[\x00-\x08\x0B\x0C\x0E-\x1F\x7F]/', '', $svg);
+	}
+
+	private function stripDangerousAttributes(DOMElement $element): void {
+		/** @var DOMAttr $attribute */
+		foreach (iterator_to_array($element->attributes) as $attribute) {
+			$name = strtolower($attribute->nodeName);
+			$localName = $this->stripNamespacePrefix($name);
+			$value = trim($attribute->nodeValue ?? '');
+
+			// Inline event handlers (onload, onclick, …).
+			if (str_starts_with($localName, 'on')) {
+				$element->removeAttributeNode($attribute);
+				continue;
+			}
+
+			// Only allow same-document references; strip javascript:, external
+			// and data: URLs from links and resource references.
+			if (in_array($localName, self::URL_ATTRIBUTES, true) && !str_starts_with($value, '#')) {
+				$element->removeAttributeNode($attribute);
+				continue;
+			}
+
+			// Strip external CSS url() references from inline style attributes.
+			if ($name === 'style') {
+				$element->setAttribute('style', $this->stripCssUrls($value));
+			}
+		}
+	}
+
+	/**
+	 * Drop the namespace prefix of an attribute name. A prefix can be bound to
+	 * any name (xlink:href, foo:href, …) and libxml keeps unresolved prefixes as
+	 * part of the node name, so matching has to happen on the local name.
+	 */
+	private function stripNamespacePrefix(string $name): string {
+		$separator = strrpos($name, ':');
+		return $separator === false ? $name : substr($name, $separator + 1);
+	}
+
+	/**
+	 * Replace CSS url() references that point outside the document with 'none'.
+	 * Fragment references (url(#…)) are preserved for gradients and masks.
+	 */
+	private function stripCssUrls(string $css): string {
+		$css = preg_replace('/@import[^;]*;?/i', '', $css) ?? $css;
+		return preg_replace('/url\s*\((?!\s*[\'\"]?#)[^)]*\)/i', 'none', $css) ?? $css;
+	}
+}