From ccb4252b582ca25e3764c284208f4207e7f78c6f Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=ABl=20de=20Jager?= <contact@joeldejager.nl>
Date: Mon, 22 Jun 2026 16:27:45 +0200
Subject: [PATCH 1/6] fix(message): render received external SVG images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

External images are fetched through the image proxy and served with a
generic application/octet-stream content type. Browsers sniff raster
formats in <img> tags, but they refuse to render SVG unless it is served
as image/svg+xml, so SVG logos stayed blank and only the alt text and
placeholder were shown.

Detect SVG markup in the proxied response and serve it as a sanitised
image/svg+xml document. A new SvgSanitizer service strips active content
(scripts, event handlers, external/javascript references) and rejects
DOCTYPE/entity declarations as XXE defence in depth, even though the
<img> context does not execute scripts.

Assisted-by: Claude:claude-opus-4-8
Signed-off-by: Joël de Jager <contact@joeldejager.nl>
---
 lib/Controller/ProxyController.php            |  30 +++++
 lib/Service/SvgSanitizer.php                  | 105 ++++++++++++++++++
 tests/Unit/Controller/ProxyControllerTest.php |  64 +++++++++++
 tests/Unit/Service/SvgSanitizerTest.php       |  82 ++++++++++++++
 4 files changed, 281 insertions(+)
 create mode 100644 lib/Service/SvgSanitizer.php
 create mode 100644 tests/Unit/Service/SvgSanitizerTest.php

diff --git a/lib/Controller/ProxyController.php b/lib/Controller/ProxyController.php
index 3b44a15254..812037b4f2 100644
--- a/lib/Controller/ProxyController.php
+++ b/lib/Controller/ProxyController.php
@@ -13,6 +13,7 @@
 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 +29,9 @@
 use Psr\Log\LoggerInterface;
 use function file_get_contents;
 use function hash_equals;
+use function ltrim;
+use function stripos;
+use function str_starts_with;
 
 #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
 class ProxyController extends Controller {
@@ -37,6 +41,7 @@ class ProxyController extends Controller {
 	private LoggerInterface $logger;
 	private ProxyHmacGenerator $hmacGenerator;
 	private MailManager $mailManager;
+	private SvgSanitizer $svgSanitizer;
 	private ?string $userId;
 
 	public function __construct(string $appName,
@@ -47,6 +52,7 @@ public function __construct(string $appName,
 		ProxyHmacGenerator $hmacGenerator,
 		LoggerInterface $logger,
 		MailManager $mailManager,
+		SvgSanitizer $svgSanitizer,
 		?string $userId) {
 		parent::__construct($appName, $request);
 		$this->request = $request;
@@ -56,6 +62,7 @@ public function __construct(string $appName,
 		$this->logger = $logger;
 		$this->hmacGenerator = $hmacGenerator;
 		$this->mailManager = $mailManager;
+		$this->svgSanitizer = $svgSanitizer;
 		$this->userId = $userId;
 	}
 
@@ -112,6 +119,29 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 			$content = file_get_contents(__DIR__ . '/../../img/blocked-image.png');
 		}
 
+		$content = (string)$content;
+
+		// 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->looksLikeSvg($content)) {
+			$content = $this->svgSanitizer->sanitize($content);
+			return new ProxyDownloadResponse($content, $src, 'image/svg+xml');
+		}
+
 		return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
 	}
+
+	/**
+	 * Heuristically decide whether the given bytes are an SVG document.
+	 */
+	private function looksLikeSvg(string $content): bool {
+		$start = ltrim($content);
+		$hasSvgPrologue = str_starts_with($start, '<?xml')
+			|| str_starts_with($start, '<!--')
+			|| stripos($start, '<svg') === 0;
+		return $hasSvgPrologue && stripos($content, '<svg') !== false;
+	}
 }
diff --git a/lib/Service/SvgSanitizer.php b/lib/Service/SvgSanitizer.php
new file mode 100644
index 0000000000..602ab1a00c
--- /dev/null
+++ b/lib/Service/SvgSanitizer.php
@@ -0,0 +1,105 @@
+<?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',
+	];
+
+	/**
+	 * @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) === '') {
+			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', $svg) === 1) {
+			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 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);
+				}
+			}
+		}
+
+		$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;
+	}
+
+	private function stripDangerousAttributes(DOMElement $element): void {
+		/** @var DOMAttr $attribute */
+		foreach (iterator_to_array($element->attributes) as $attribute) {
+			$name = strtolower($attribute->nodeName);
+			$value = trim($attribute->nodeValue ?? '');
+
+			// Inline event handlers (onload, onclick, …).
+			if (str_starts_with($name, 'on')) {
+				$element->removeAttributeNode($attribute);
+				continue;
+			}
+
+			// Only allow same-document references; strip javascript:, external
+			// and data: URLs from links and resource references.
+			if (in_array($name, ['href', 'xlink:href'], true) && !str_starts_with($value, '#')) {
+				$element->removeAttributeNode($attribute);
+			}
+		}
+	}
+}
diff --git a/tests/Unit/Controller/ProxyControllerTest.php b/tests/Unit/Controller/ProxyControllerTest.php
index ee6ed5cf0a..0c956fb6b7 100644
--- a/tests/Unit/Controller/ProxyControllerTest.php
+++ b/tests/Unit/Controller/ProxyControllerTest.php
@@ -15,6 +15,7 @@
 use OCA\Mail\Html\ProxyHmacGenerator;
 use OCA\Mail\Http\ProxyDownloadResponse;
 use OCA\Mail\Service\MailManager;
+use OCA\Mail\Service\SvgSanitizer;
 use OCP\AppFramework\Db\DoesNotExistException;
 use OCP\AppFramework\Http;
 use OCP\AppFramework\Http\Response;
@@ -53,6 +54,9 @@ class ProxyControllerTest extends TestCase {
 	/** @var MailManager|MockObject */
 	private $mailManager;
 
+	/** @var SvgSanitizer|MockObject */
+	private $svgSanitizer;
+
 	private string $userId = 'user';
 
 	/** @var ProxyController */
@@ -68,6 +72,7 @@ protected function setUp(): void {
 		$this->clientService = $this->createMock(IClientService::class);
 		$this->hmacGenerator = $this->createMock(ProxyHmacGenerator::class);
 		$this->mailManager = $this->createMock(MailManager::class);
+		$this->svgSanitizer = $this->createMock(SvgSanitizer::class);
 		$this->logger = new NullLogger();
 	}
 
@@ -90,6 +95,7 @@ public function testProxyWithoutCookies(): void {
 			$this->hmacGenerator,
 			$this->logger,
 			$this->mailManager,
+			$this->svgSanitizer,
 			$this->userId,
 		);
 
@@ -136,12 +142,67 @@ public function testProxy(): void {
 			$this->hmacGenerator,
 			$this->logger,
 			$this->mailManager,
+			$this->svgSanitizer,
+			$this->userId,
+		);
+
+		$response = $this->controller->proxy($src, $id, $validHmac);
+
+		$this->assertInstanceOf(ProxyDownloadResponse::class, $response);
+	}
+
+	public function testProxySanitizesAndServesSvg(): void {
+		$src = 'https://example.com/logo.svg';
+		$id = 1;
+		$validHmac = 'valid-hmac-hash';
+		$content = '<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script></svg>';
+		$sanitized = '<svg xmlns="http://www.w3.org/2000/svg"></svg>';
+		$httpResponse = $this->createMock(IResponse::class);
+		$this->request->expects(self::once())
+			->method('passesStrictCookieCheck')
+			->willReturn(true);
+		$this->session->expects($this->once())
+			->method('close');
+		$this->hmacGenerator->expects($this->once())
+			->method('generate')
+			->with($id, $src)
+			->willReturn($validHmac);
+		$this->mailManager->expects($this->once())
+			->method('getMessage')
+			->with($this->userId, $id);
+		$client = $this->getMockBuilder(IClient::class)->getMock();
+		$this->clientService->expects($this->once())
+			->method('newClient')
+			->willReturn($client);
+		$client->expects($this->once())
+			->method('get')
+			->with($src)
+			->willReturn($httpResponse);
+		$httpResponse->expects($this->once())
+			->method('getBody')
+			->willReturn($content);
+		$this->svgSanitizer->expects($this->once())
+			->method('sanitize')
+			->with($content)
+			->willReturn($sanitized);
+		$this->controller = new ProxyController(
+			$this->appName,
+			$this->request,
+			$this->urlGenerator,
+			$this->session,
+			$this->clientService,
+			$this->hmacGenerator,
+			$this->logger,
+			$this->mailManager,
+			$this->svgSanitizer,
 			$this->userId,
 		);
 
 		$response = $this->controller->proxy($src, $id, $validHmac);
 
 		$this->assertInstanceOf(ProxyDownloadResponse::class, $response);
+		$this->assertSame('image/svg+xml', $response->getHeaders()['Content-Type']);
+		$this->assertSame($sanitized, $response->render());
 	}
 
 	public function testProxyWithInvalidHmac(): void {
@@ -172,6 +233,7 @@ public function testProxyWithInvalidHmac(): void {
 			$this->hmacGenerator,
 			$this->logger,
 			$this->mailManager,
+			$this->svgSanitizer,
 			$this->userId,
 		);
 
@@ -199,6 +261,7 @@ public function testProxyWithMissingHmacParameters(): void {
 			$this->hmacGenerator,
 			$this->logger,
 			$this->mailManager,
+			$this->svgSanitizer,
 			$this->userId,
 		);
 
@@ -231,6 +294,7 @@ public function testProxyWithMessageNotOwnedByUser(): void {
 			$this->hmacGenerator,
 			$this->logger,
 			$this->mailManager,
+			$this->svgSanitizer,
 			$this->userId,
 		);
 
diff --git a/tests/Unit/Service/SvgSanitizerTest.php b/tests/Unit/Service/SvgSanitizerTest.php
new file mode 100644
index 0000000000..f4d576a9e9
--- /dev/null
+++ b/tests/Unit/Service/SvgSanitizerTest.php
@@ -0,0 +1,82 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * SPDX-FileCopyrightText: 2026 Nextcloud GmbH and Nextcloud contributors
+ * SPDX-License-Identifier: AGPL-3.0-or-later
+ */
+
+namespace OCA\Mail\Tests\Unit\Service;
+
+use ChristophWurst\Nextcloud\Testing\TestCase;
+use OCA\Mail\Service\SvgSanitizer;
+
+class SvgSanitizerTest extends TestCase {
+	private SvgSanitizer $sanitizer;
+
+	protected function setUp(): void {
+		parent::setUp();
+
+		$this->sanitizer = new SvgSanitizer();
+	}
+
+	public function testRemovesScript(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg"><script>alert(1)</script><rect/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringNotContainsString('<script', $result);
+		$this->assertStringContainsString('<rect', $result);
+	}
+
+	public function testRemovesEventHandlers(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg"><rect onload="evil()" width="10"/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringNotContainsString('onload', $result);
+		$this->assertStringContainsString('width="10"', $result);
+	}
+
+	public function testRemovesExternalReference(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">'
+			. '<a xlink:href="https://evil.example"><rect/></a></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringNotContainsString('evil.example', $result);
+		$this->assertStringContainsString('<rect', $result);
+	}
+
+	public function testKeepsSameDocumentReference(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg" xmlns:xlink="http://www.w3.org/1999/xlink">'
+			. '<use xlink:href="#icon"/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringContainsString('#icon', $result);
+	}
+
+	public function testRejectsDoctypeAndEntities(): void {
+		$svg = '<!DOCTYPE svg [<!ENTITY xxe SYSTEM "file:///etc/passwd">]>'
+			. '<svg xmlns="http://www.w3.org/2000/svg"><rect/></svg>';
+
+		$this->assertSame('', $this->sanitizer->sanitize($svg));
+	}
+
+	public function testReturnsEmptyForInvalidMarkup(): void {
+		$this->assertSame('', $this->sanitizer->sanitize('this is not <<< svg'));
+		$this->assertSame('', $this->sanitizer->sanitize(''));
+	}
+
+	public function testKeepsSafeGraphics(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg" viewBox="0 0 10 10">'
+			. '<circle cx="5" cy="5" r="4" fill="red"/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringContainsString('<circle', $result);
+		$this->assertStringContainsString('fill="red"', $result);
+	}
+}

From f9279d60e804e8d96ec60eaaa6727248b2bb35ce Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=ABl=20de=20Jager?= <contact@joeldejager.nl>
Date: Tue, 23 Jun 2026 13:31:46 +0200
Subject: [PATCH 2/6] fix(proxy): close SVG sanitizer security gaps
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

- Extend URL attribute stripping to cover src, action, formaction
- Strip external CSS url() references from <style> elements and style=""
  attributes while preserving fragment references (url(#...))
- Reject SVG payloads exceeding 2 MiB before DOM parsing (DoS guard)
- Fall back to blocked-image.png instead of serving empty body when
  sanitizer rejects an SVG document
- Drop misleading <!-- branch from looksLikeSvg() heuristic
- Move looksLikeSvg() from ProxyController into SvgSanitizer service

Assisted-by: Claude:claude-sonnet-4-6
Signed-off-by: Joël de Jager <contact@joeldejager.nl>
---
 lib/Controller/ProxyController.php            | 24 ++----
 lib/Service/SvgSanitizer.php                  | 42 +++++++++-
 tests/Unit/Controller/ProxyControllerTest.php | 60 +++++++++++++++
 tests/Unit/Service/SvgSanitizerTest.php       | 77 +++++++++++++++++++
 4 files changed, 184 insertions(+), 19 deletions(-)

diff --git a/lib/Controller/ProxyController.php b/lib/Controller/ProxyController.php
index 812037b4f2..a6a132285b 100644
--- a/lib/Controller/ProxyController.php
+++ b/lib/Controller/ProxyController.php
@@ -29,9 +29,6 @@
 use Psr\Log\LoggerInterface;
 use function file_get_contents;
 use function hash_equals;
-use function ltrim;
-use function stripos;
-use function str_starts_with;
 
 #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
 class ProxyController extends Controller {
@@ -126,22 +123,15 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 		// 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->looksLikeSvg($content)) {
-			$content = $this->svgSanitizer->sanitize($content);
-			return new ProxyDownloadResponse($content, $src, 'image/svg+xml');
+		if ($this->svgSanitizer->looksLikeSvg($content)) {
+			$sanitized = $this->svgSanitizer->sanitize($content);
+			if ($sanitized === '') {
+				$content = (string)file_get_contents(__DIR__ . '/../../img/blocked-image.png');
+				return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
+			}
+			return new ProxyDownloadResponse($sanitized, $src, 'image/svg+xml');
 		}
 
 		return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
 	}
-
-	/**
-	 * Heuristically decide whether the given bytes are an SVG document.
-	 */
-	private function looksLikeSvg(string $content): bool {
-		$start = ltrim($content);
-		$hasSvgPrologue = str_starts_with($start, '<?xml')
-			|| str_starts_with($start, '<!--')
-			|| stripos($start, '<svg') === 0;
-		return $hasSvgPrologue && stripos($content, '<svg') !== false;
-	}
 }
diff --git a/lib/Service/SvgSanitizer.php b/lib/Service/SvgSanitizer.php
index 602ab1a00c..408e08afe8 100644
--- a/lib/Service/SvgSanitizer.php
+++ b/lib/Service/SvgSanitizer.php
@@ -30,13 +30,19 @@ class SvgSanitizer {
 		'set',
 	];
 
+	/** Attributes that carry URL references and must not point off-document. */
+	private const URL_ATTRIBUTES = ['href', 'xlink:href', 'src', 'action', 'formaction'];
+
+	/** 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) === '') {
+		if (trim($svg) === '' || strlen($svg) > self::MAX_SVG_BYTES) {
 			return '';
 		}
 
@@ -70,6 +76,14 @@ public function sanitize(string $svg): string {
 			}
 		}
 
+		// 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) {
@@ -83,6 +97,16 @@ public function sanitize(string $svg): string {
 		return $result === false ? '' : $result;
 	}
 
+	/**
+	 * Heuristically decide whether the given bytes are an SVG document.
+	 */
+	public function looksLikeSvg(string $content): bool {
+		$start = ltrim($content);
+		$hasSvgPrologue = str_starts_with($start, '<?xml')
+			|| stripos($start, '<svg') === 0;
+		return $hasSvgPrologue && stripos($content, '<svg') !== false;
+	}
+
 	private function stripDangerousAttributes(DOMElement $element): void {
 		/** @var DOMAttr $attribute */
 		foreach (iterator_to_array($element->attributes) as $attribute) {
@@ -97,9 +121,23 @@ private function stripDangerousAttributes(DOMElement $element): void {
 
 			// Only allow same-document references; strip javascript:, external
 			// and data: URLs from links and resource references.
-			if (in_array($name, ['href', 'xlink:href'], true) && !str_starts_with($value, '#')) {
+			if (in_array($name, 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));
 			}
 		}
 	}
+
+	/**
+	 * 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 {
+		return preg_replace('/url\s*\((?!\s*[\'"]?#)[^)]*\)/i', 'none', $css) ?? $css;
+	}
 }
diff --git a/tests/Unit/Controller/ProxyControllerTest.php b/tests/Unit/Controller/ProxyControllerTest.php
index 0c956fb6b7..cb78fd1735 100644
--- a/tests/Unit/Controller/ProxyControllerTest.php
+++ b/tests/Unit/Controller/ProxyControllerTest.php
@@ -181,6 +181,10 @@ public function testProxySanitizesAndServesSvg(): void {
 		$httpResponse->expects($this->once())
 			->method('getBody')
 			->willReturn($content);
+		$this->svgSanitizer->expects($this->once())
+			->method('looksLikeSvg')
+			->with($content)
+			->willReturn(true);
 		$this->svgSanitizer->expects($this->once())
 			->method('sanitize')
 			->with($content)
@@ -205,6 +209,62 @@ public function testProxySanitizesAndServesSvg(): void {
 		$this->assertSame($sanitized, $response->render());
 	}
 
+	public function testProxyServesBlockedImageWhenSvgSanitizerReturnsEmpty(): void {
+		$src = 'https://example.com/malicious.svg';
+		$id = 1;
+		$validHmac = 'valid-hmac-hash';
+		$content = '<svg xmlns="http://www.w3.org/2000/svg"><!DOCTYPE evil></svg>';
+		$httpResponse = $this->createMock(IResponse::class);
+		$this->request->expects(self::once())
+			->method('passesStrictCookieCheck')
+			->willReturn(true);
+		$this->session->expects($this->once())
+			->method('close');
+		$this->hmacGenerator->expects($this->once())
+			->method('generate')
+			->with($id, $src)
+			->willReturn($validHmac);
+		$this->mailManager->expects($this->once())
+			->method('getMessage')
+			->with($this->userId, $id);
+		$client = $this->getMockBuilder(IClient::class)->getMock();
+		$this->clientService->expects($this->once())
+			->method('newClient')
+			->willReturn($client);
+		$client->expects($this->once())
+			->method('get')
+			->with($src)
+			->willReturn($httpResponse);
+		$httpResponse->expects($this->once())
+			->method('getBody')
+			->willReturn($content);
+		$this->svgSanitizer->expects($this->once())
+			->method('looksLikeSvg')
+			->with($content)
+			->willReturn(true);
+		$this->svgSanitizer->expects($this->once())
+			->method('sanitize')
+			->with($content)
+			->willReturn('');
+		$this->controller = new ProxyController(
+			$this->appName,
+			$this->request,
+			$this->urlGenerator,
+			$this->session,
+			$this->clientService,
+			$this->hmacGenerator,
+			$this->logger,
+			$this->mailManager,
+			$this->svgSanitizer,
+			$this->userId,
+		);
+
+		$response = $this->controller->proxy($src, $id, $validHmac);
+
+		$this->assertInstanceOf(ProxyDownloadResponse::class, $response);
+		$this->assertSame('application/octet-stream', $response->getHeaders()['Content-Type']);
+	}
+
 	public function testProxyWithInvalidHmac(): void {
 		$src = 'http://example.com';
 		$id = 1;
diff --git a/tests/Unit/Service/SvgSanitizerTest.php b/tests/Unit/Service/SvgSanitizerTest.php
index f4d576a9e9..937f3cb8f7 100644
--- a/tests/Unit/Service/SvgSanitizerTest.php
+++ b/tests/Unit/Service/SvgSanitizerTest.php
@@ -79,4 +79,81 @@ public function testKeepsSafeGraphics(): void {
 		$this->assertStringContainsString('<circle', $result);
 		$this->assertStringContainsString('fill="red"', $result);
 	}
+
+	public function testRemovesSrcAttribute(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg">'
+			. '<image src="https://tracker.example/pixel.gif" width="1" height="1"/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringNotContainsString('tracker.example', $result);
+		$this->assertStringContainsString('width="1"', $result);
+	}
+
+	public function testStripsExternalUrlFromStyleAttribute(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg">'
+			. '<rect style="fill: url(https://tracker.example/img.png)" width="10"/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringNotContainsString('tracker.example', $result);
+		$this->assertStringContainsString('width="10"', $result);
+	}
+
+	public function testKeepsSameDocumentUrlInStyleAttribute(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg">'
+			. '<defs><linearGradient id="g"><stop offset="0" stop-color="red"/></linearGradient></defs>'
+			. '<rect style="fill: url(#g)" width="10"/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringContainsString('url(#g)', $result);
+	}
+
+	public function testStripsExternalUrlFromStyleElement(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg">'
+			. '<style>rect { fill: url(https://tracker.example/img.png) }</style>'
+			. '<rect width="10"/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringNotContainsString('tracker.example', $result);
+		$this->assertStringContainsString('<rect', $result);
+	}
+
+	public function testKeepsSafeStyleRules(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg">'
+			. '<style>rect { fill: red; stroke: blue }</style>'
+			. '<rect width="10"/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringContainsString('fill: red', $result);
+	}
+
+	public function testRejectsOversizedInput(): void {
+		$svg = str_repeat('a', 2 * 1024 * 1024 + 1);
+
+		$this->assertSame('', $this->sanitizer->sanitize($svg));
+	}
+
+	public function testLooksLikeSvgWithDirectSvgTag(): void {
+		$this->assertTrue($this->sanitizer->looksLikeSvg('<svg xmlns="http://www.w3.org/2000/svg"/>'));
+	}
+
+	public function testLooksLikeSvgWithXmlPrologue(): void {
+		$this->assertTrue($this->sanitizer->looksLikeSvg('<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg"/>'));
+	}
+
+	public function testLooksLikeSvgReturnsFalseForHtml(): void {
+		$this->assertFalse($this->sanitizer->looksLikeSvg('<!DOCTYPE html><html><body><svg/></body></html>'));
+	}
+
+	public function testLooksLikeSvgReturnsFalseForRasterImage(): void {
+		$this->assertFalse($this->sanitizer->looksLikeSvg("\x89PNG\r\n"));
+	}
+
+	public function testLooksLikeSvgReturnsFalseForHtmlComment(): void {
+		$this->assertFalse($this->sanitizer->looksLikeSvg('<!-- comment --><svg/>'));
+	}
 }

From 80a7a9efc8cc63543d572381be3efb23d421781d Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=ABl=20de=20Jager?= <contact@joeldejager.nl>
Date: Tue, 21 Jul 2026 22:00:20 +0200
Subject: [PATCH 3/6] fix(proxy): handle blocked image read failures
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Assisted-by: Codex:gpt-5
Signed-off-by: Joël de Jager <contact@joeldejager.nl>
---
 lib/Controller/ProxyController.php | 22 ++++++++++++++++++----
 1 file changed, 18 insertions(+), 4 deletions(-)

diff --git a/lib/Controller/ProxyController.php b/lib/Controller/ProxyController.php
index a6a132285b..cf7e8227d7 100644
--- a/lib/Controller/ProxyController.php
+++ b/lib/Controller/ProxyController.php
@@ -10,6 +10,7 @@
 
 namespace OCA\Mail\Controller;
 
+use OCA\Mail\Exception\ServiceException;
 use OCA\Mail\Html\ProxyHmacGenerator;
 use OCA\Mail\Http\ProxyDownloadResponse;
 use OCA\Mail\Service\MailManager;
@@ -75,6 +76,7 @@ public function __construct(string $appName,
 	 *       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 {
@@ -83,7 +85,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');
 		}
 
@@ -107,13 +109,13 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 			$content = $response->getBody();
 		} 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();
 		}
 
 		$content = (string)$content;
@@ -126,7 +128,7 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 		if ($this->svgSanitizer->looksLikeSvg($content)) {
 			$sanitized = $this->svgSanitizer->sanitize($content);
 			if ($sanitized === '') {
-				$content = (string)file_get_contents(__DIR__ . '/../../img/blocked-image.png');
+				$content = $this->getBlockedImage();
 				return new ProxyDownloadResponse($content, $src, 'application/octet-stream');
 			}
 			return new ProxyDownloadResponse($sanitized, $src, 'image/svg+xml');
@@ -134,4 +136,16 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 
 		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;
+	}
 }

From a47fda59610d61c954b18ccb4d7941d22cb95f52 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=ABl=20de=20Jager?= <contact@joeldejager.nl>
Date: Wed, 22 Jul 2026 11:44:52 +0200
Subject: [PATCH 4/6] fixup! fix(message): render received external SVG images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Assisted-by: Codex:gpt-5
Signed-off-by: Joël de Jager <contact@joeldejager.nl>
---
 lib/Controller/ProxyController.php | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/lib/Controller/ProxyController.php b/lib/Controller/ProxyController.php
index f4a3b872af..955bb4fbbe 100644
--- a/lib/Controller/ProxyController.php
+++ b/lib/Controller/ProxyController.php
@@ -30,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 {
@@ -46,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);
@@ -98,6 +100,9 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 		try {
 			$response = $client->get($src);
 			$content = $response->getBody();
+			if (!is_string($content)) {
+				throw new ServiceException('Could not read proxied image');
+			}
 		} catch (ClientExceptionInterface $e) {
 			$this->logger->notice('Unable to proxy image', ['exception' => $e]);
 			$content = $this->getBlockedImage();
@@ -109,8 +114,6 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 			$content = $this->getBlockedImage();
 		}
 
-		$content = (string)$content;
-
 		// 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

From 431b9bfe0797c22ddc884675a5e12ca53d55e830 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=ABl=20de=20Jager?= <contact@joeldejager.nl>
Date: Wed, 22 Jul 2026 14:37:15 +0200
Subject: [PATCH 5/6] fixup! fix(message): render received external SVG images
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Assisted-by: Codex:gpt-5
Signed-off-by: Joël de Jager <contact@joeldejager.nl>
---
 lib/Controller/ProxyController.php            | 10 +++++++---
 lib/Service/SvgSanitizer.php                  |  3 +++
 tests/Unit/Controller/ProxyControllerTest.php | 10 ++++++++--
 tests/Unit/Service/SvgSanitizerTest.php       |  4 ++++
 4 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/lib/Controller/ProxyController.php b/lib/Controller/ProxyController.php
index 955bb4fbbe..ab591843f5 100644
--- a/lib/Controller/ProxyController.php
+++ b/lib/Controller/ProxyController.php
@@ -30,7 +30,8 @@
 use Psr\Log\LoggerInterface;
 use function file_get_contents;
 use function hash_equals;
-use function is_string;
+use function is_resource;
+use function stream_get_contents;
 
 #[OpenAPI(scope: OpenAPI::SCOPE_IGNORE)]
 class ProxyController extends Controller {
@@ -100,8 +101,11 @@ public function proxy(string $src, ?int $id, ?string $hmac): Response {
 		try {
 			$response = $client->get($src);
 			$content = $response->getBody();
-			if (!is_string($content)) {
-				throw new ServiceException('Could not read proxied image');
+			if (is_resource($content)) {
+				$content = stream_get_contents($content);
+				if ($content === false) {
+					$content = $this->getBlockedImage();
+				}
 			}
 		} catch (ClientExceptionInterface $e) {
 			$this->logger->notice('Unable to proxy image', ['exception' => $e]);
diff --git a/lib/Service/SvgSanitizer.php b/lib/Service/SvgSanitizer.php
index 408e08afe8..87a34ef401 100644
--- a/lib/Service/SvgSanitizer.php
+++ b/lib/Service/SvgSanitizer.php
@@ -102,6 +102,9 @@ public function sanitize(string $svg): string {
 	 */
 	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;
diff --git a/tests/Unit/Controller/ProxyControllerTest.php b/tests/Unit/Controller/ProxyControllerTest.php
index c562ef7ed3..6d528fe91c 100644
--- a/tests/Unit/Controller/ProxyControllerTest.php
+++ b/tests/Unit/Controller/ProxyControllerTest.php
@@ -103,11 +103,15 @@ public function testProxyWithoutCookies(): void {
 		$this->assertInstanceOf(ProxyDownloadResponse::class, $response);
 	}
 
-	public function testProxy(): void {
+	public function testProxyReadsResourceBody(): void {
 		$src = 'http://example.com';
 		$id = 1;
 		$validHmac = 'valid-hmac-hash';
 		$content = '🐵🐵🐵';
+		$stream = fopen('php://memory', 'r+');
+		$this->assertIsResource($stream);
+		fwrite($stream, $content);
+		rewind($stream);
 		$httpResponse = $this->createMock(IResponse::class);
 		$this->request->expects(self::once())
 			->method('passesStrictCookieCheck')
@@ -131,7 +135,7 @@ public function testProxy(): void {
 			->willReturn($httpResponse);
 		$httpResponse->expects($this->once())
 			->method('getBody')
-			->willReturn($content);
+			->willReturn($stream);
 		$this->controller = new ProxyController(
 			$this->appName,
 			$this->request,
@@ -146,8 +150,10 @@ public function testProxy(): void {
 		);
 
 		$response = $this->controller->proxy($src, $id, $validHmac);
+		fclose($stream);
 
 		$this->assertInstanceOf(ProxyDownloadResponse::class, $response);
+		$this->assertSame($content, $response->render());
 	}
 
 	public function testProxySanitizesAndServesSvg(): void {
diff --git a/tests/Unit/Service/SvgSanitizerTest.php b/tests/Unit/Service/SvgSanitizerTest.php
index 937f3cb8f7..65cb4d4fd7 100644
--- a/tests/Unit/Service/SvgSanitizerTest.php
+++ b/tests/Unit/Service/SvgSanitizerTest.php
@@ -145,6 +145,10 @@ public function testLooksLikeSvgWithXmlPrologue(): void {
 		$this->assertTrue($this->sanitizer->looksLikeSvg('<?xml version="1.0"?><svg xmlns="http://www.w3.org/2000/svg"/>'));
 	}
 
+	public function testLooksLikeSvgWithUtf8Bom(): void {
+		$this->assertTrue($this->sanitizer->looksLikeSvg("\xEF\xBB\xBF<svg xmlns=\"http://www.w3.org/2000/svg\"/>"));
+	}
+
 	public function testLooksLikeSvgReturnsFalseForHtml(): void {
 		$this->assertFalse($this->sanitizer->looksLikeSvg('<!DOCTYPE html><html><body><svg/></body></html>'));
 	}

From c637d51a3af9348ec8328ab3106505d8726a5cec Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Jo=C3=ABl=20de=20Jager?= <contact@joeldejager.nl>
Date: Fri, 24 Jul 2026 20:30:40 +0200
Subject: [PATCH 6/6] fixup! fix(proxy): close SVG sanitizer security gaps
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Signed-off-by: Joël de Jager <contact@joeldejager.nl>
---
 lib/Service/SvgSanitizer.php            | 17 +++++++++++++++++
 tests/Unit/Service/SvgSanitizerTest.php | 21 +++++++++++++++++++++
 2 files changed, 38 insertions(+)

diff --git a/lib/Service/SvgSanitizer.php b/lib/Service/SvgSanitizer.php
index 87a34ef401..06dd995000 100644
--- a/lib/Service/SvgSanitizer.php
+++ b/lib/Service/SvgSanitizer.php
@@ -52,6 +52,13 @@ public function sanitize(string $svg): string {
 			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($svg, '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.
@@ -65,6 +72,16 @@ public function sanitize(string $svg): string {
 
 		$xpath = new DOMXPath($dom);
 
+		// Remove processing instructions (e.g. <?xml-stylesheet type="text/xsl"?>).
+		// 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) {
diff --git a/tests/Unit/Service/SvgSanitizerTest.php b/tests/Unit/Service/SvgSanitizerTest.php
index 65cb4d4fd7..e8aef687cb 100644
--- a/tests/Unit/Service/SvgSanitizerTest.php
+++ b/tests/Unit/Service/SvgSanitizerTest.php
@@ -131,6 +131,27 @@ public function testKeepsSafeStyleRules(): void {
 		$this->assertStringContainsString('fill: red', $result);
 	}
 
+	public function testRejectsXslTransformNamespace(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg"'
+			. ' xmlns:xsl="http://www.w3.org/1999/XSL/Transform">'
+			. '<xsl:stylesheet version="1.0"><xsl:template match="/">'
+			. '<script>alert(1)</script></xsl:template></xsl:stylesheet>'
+			. '</svg>';
+
+		$this->assertSame('', $this->sanitizer->sanitize($svg));
+	}
+
+	public function testStripsProcessingInstructionsInsideRoot(): void {
+		$svg = '<svg xmlns="http://www.w3.org/2000/svg">'
+			. '<?xml-stylesheet type="text/css" href="style.css"?>'
+			. '<rect width="10"/></svg>';
+
+		$result = $this->sanitizer->sanitize($svg);
+
+		$this->assertStringNotContainsString('<?xml-stylesheet', $result);
+		$this->assertStringContainsString('<rect', $result);
+	}
+
 	public function testRejectsOversizedInput(): void {
 		$svg = str_repeat('a', 2 * 1024 * 1024 + 1);