diff --git a/lib/Controller/ProxyController.php b/lib/Controller/ProxyController.php index 3b44a15254..a6a132285b 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; @@ -37,6 +38,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 +49,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 +59,7 @@ public function __construct(string $appName, $this->logger = $logger; $this->hmacGenerator = $hmacGenerator; $this->mailManager = $mailManager; + $this->svgSanitizer = $svgSanitizer; $this->userId = $userId; } @@ -112,6 +116,22 @@ 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 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-) navigation. + 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'); } } diff --git a/lib/Service/SvgSanitizer.php b/lib/Service/SvgSanitizer.php new file mode 100644 index 0000000000..408e08afe8 --- /dev/null +++ b/lib/Service/SvgSanitizer.php @@ -0,0 +1,143 @@ +/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', + ]; + + /** 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) === '' || strlen($svg) > self::MAX_SVG_BYTES) { + 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('/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. ). + 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 ' + . ''; + + $result = $this->sanitizer->sanitize($svg); + + $this->assertStringNotContainsString('tracker.example', $result); + $this->assertStringContainsString('rect { fill: red; stroke: blue }' + . ''; + + $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('')); + } + + public function testLooksLikeSvgWithXmlPrologue(): void { + $this->assertTrue($this->sanitizer->looksLikeSvg('')); + } + + public function testLooksLikeSvgReturnsFalseForHtml(): void { + $this->assertFalse($this->sanitizer->looksLikeSvg('')); + } + + public function testLooksLikeSvgReturnsFalseForRasterImage(): void { + $this->assertFalse($this->sanitizer->looksLikeSvg("\x89PNG\r\n")); + } + + public function testLooksLikeSvgReturnsFalseForHtmlComment(): void { + $this->assertFalse($this->sanitizer->looksLikeSvg('')); + } +}