[SECURITY] Harden message deserialization in FileSpool transport

eliashaeussler · ohader · commit e0f0ceee480c · 2026-01-13T12:33:25.000+01:00
Messages used for delayed transport using the `FileSpool` transport are serialized `SentMessage` object representations. Deserializing queued messages must therefore accept only `SentMessage` and related classes. An existing security measure to limit deserialization to said classes is already in place, but contained a typo. Hence, the security measures did not have any effect. This is now fixed and all explicitly allowed classes are now properly configured for the `unserialize` command. However, since the current `symfony/mailer` implementations are constructed to include a various range of related classes, which may also be extended by custom implementations, it's quite hard to list all allowed classes in a static list. In order to cover this specific scenario, a new `PolymorphicDeserializer` component is introduced. It performs an extended deserialization, which is able to construct the list of allowed classes by inspecting the serialized payload: All extracted objects must match a predefined list of allowed classes or at least implement any of the listed interfaces, otherwise deserialization will fail. The `PolymorphicDeserializer` component is now consumed by the `FileSpool` transport. In addition, the `flushQueue` operation was slightly adjusted to enforce `SentMessage` objects and skip other serialized messages. In case any serialized messages contain disallowed or unsupported objects, an error will now be logged. The serialized message will be removed in any case – either after transport or when being skipped due to the described behavior – to avoid having invalid messages in place. Resolves: #108610 Releases: main, 14.0, 13.4, 12.4 Change-Id: I22db2d7f0ff46d51d15c76e61296c0ef8ac0e23c Security-Bulletin: TYPO3-CORE-SA-2026-004 Security-References: CVE-2026-0859 Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/92302 Reviewed-by: Oliver Hader <oliver.hader@typo3.org> Tested-by: Oliver Hader <oliver.hader@typo3.org>
diff --git a/typo3/sysext/core/Classes/Mail/FileSpool.php b/typo3/sysext/core/Classes/Mail/FileSpool.php
@@ -18,16 +18,17 @@
 namespace TYPO3\CMS\Core\Mail;
 
 use Psr\Log\LoggerInterface;
-use Symfony\Component\Mailer\DelayedEnvelope;
 use Symfony\Component\Mailer\Envelope;
 use Symfony\Component\Mailer\Exception\TransportException;
 use Symfony\Component\Mailer\SentMessage;
 use Symfony\Component\Mailer\Transport\AbstractTransport;
 use Symfony\Component\Mailer\Transport\TransportInterface;
-use Symfony\Component\Mime\Email;
-use Symfony\Component\Mime\Message;
+use Symfony\Component\Mime\Address;
+use Symfony\Component\Mime\Header\HeaderInterface;
+use Symfony\Component\Mime\Header\Headers;
 use Symfony\Component\Mime\RawMessage;
 use Symfony\Contracts\EventDispatcher\EventDispatcherInterface;
+use TYPO3\CMS\Core\Serializer\PolymorphicDeserializer;
 use TYPO3\CMS\Core\Utility\GeneralUtility;
 
 /**
@@ -58,7 +59,8 @@ class FileSpool extends AbstractTransport implements DelayedTransportInterface
     public function __construct(
         protected string $path,
         ?EventDispatcherInterface $dispatcher = null,
-        protected readonly ?LoggerInterface $logger = null
+        protected readonly ?LoggerInterface $logger = null,
+        protected readonly PolymorphicDeserializer $deserializer = new PolymorphicDeserializer(),
     ) {
         parent::__construct($dispatcher, $logger);
 
@@ -140,20 +142,39 @@ public function flushQueue(TransportInterface $transport): int
 
             /* We try a rename, it's an atomic operation, and avoid locking the file */
             if (rename($file, $file . '.sending')) {
-                $message = unserialize((string)file_get_contents($file . '.sending'), [
-                    'allowedClasses' => [
-                        RawMessage::class,
-                        Message::class,
-                        Email::class,
-                        DelayedEnvelope::class,
-                        Envelope::class,
-                    ],
-                ]);
-
-                $transport->send($message->getMessage(), $message->getEnvelope());
-                $count++;
-
-                unlink($file . '.sending');
+                try {
+                    $message = $this->deserializer->deserialize(
+                        (string)file_get_contents($file . '.sending'),
+                        [
+                            SentMessage::class,
+                            RawMessage::class,
+                            Envelope::class,
+                            Address::class,
+                            Headers::class,
+                            HeaderInterface::class,
+                        ]
+                    );
+
+                    if ($message instanceof SentMessage) {
+                        $transport->send($message->getMessage(), $message->getEnvelope());
+                        $count++;
+                    } else {
+                        $this->logger?->error(
+                            'Serialized message from {fileName} was rejected, because {className} is not an instance of SentMessage.',
+                            [
+                                'fileName' => $file,
+                                'className' => get_debug_type($message),
+                            ],
+                        );
+                    }
+                } catch (\Throwable) {
+                    $this->logger?->error(
+                        'Serialized message from {fileName} was rejected, because it contains a disallowed class object.',
+                        ['fileName' => $file],
+                    );
+                } finally {
+                    unlink($file . '.sending');
+                }
             } else {
                 /* This message has just been caught by another process */
                 continue;
diff --git a/typo3/sysext/core/Classes/Serializer/Exception/PolymorphicDeserializerException.php b/typo3/sysext/core/Classes/Serializer/Exception/PolymorphicDeserializerException.php
@@ -0,0 +1,27 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Serializer\Exception;
+
+use TYPO3\CMS\Core\Exception;
+
+/**
+ * An exception if de-serializing an object failed
+ *
+ * @internal
+ */
+class PolymorphicDeserializerException extends Exception {}
diff --git a/typo3/sysext/core/Classes/Serializer/PolymorphicDeserializer.php b/typo3/sysext/core/Classes/Serializer/PolymorphicDeserializer.php
@@ -0,0 +1,118 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Serializer;
+
+use TYPO3\CMS\Core\Serializer\Exception\PolymorphicDeserializerException;
+
+/**
+ * @internal Only to be used by TYPO3 core
+ */
+final readonly class PolymorphicDeserializer
+{
+    /**
+     * Validates the serialized payload by checking a static list of base classes or interfaces to be included in the
+     * de-serialized output. If a non-allowed class is hit, the method throws an PolymorphicDeserializerException.
+     * If the serialized payload is syntactically incorrect, PolymorphicDeserializerException is thrown as well.
+     *
+     * @param list<class-string> $allowedClasses
+     * @throws PolymorphicDeserializerException
+     */
+    public function deserialize(string $payload, array $allowedClasses): mixed
+    {
+        // When allowing inheritance, extract all class names from payload and validate them
+        $classNames = $this->parseClassNames($payload);
+
+        foreach ($classNames as $className) {
+            if (!$this->isInstanceOf($className, $allowedClasses)) {
+                throw new PolymorphicDeserializerException('Invalid class name "' . $className . '" found in payload', 1767987405);
+            }
+
+            // Add the class if it's a valid subclass of any allowed class
+            if (!in_array($className, $allowedClasses, true)) {
+                $allowedClasses[] = $className;
+            }
+        }
+
+        $result = @unserialize($payload, ['allowed_classes' => $allowedClasses]);
+        if ($result === false) {
+            if ($payload === serialize(false)) {
+                // Do not throw an exception in case the serialized string is *actually* false
+                // See https://www.php.net/manual/en/function.unserialize.php#refsect1-function.unserialize-notes
+                return false;
+            }
+            $exceptionMessage = 'Syntax error in payload, unable to de-serialize';
+            $lastError = error_get_last();
+            if ($lastError !== null) {
+                $exceptionMessage .= ': ' . $lastError['message'];
+            }
+            throw new PolymorphicDeserializerException($exceptionMessage, 1768212616);
+        }
+
+        return $result;
+    }
+
+    public function parseClassNames(string $payload): array
+    {
+        $classNames = [];
+        if (preg_match_all('/[CO]:(?P<length>\d+):"(?P<className>[^"]+)"/', $payload, $matches, PREG_OFFSET_CAPTURE)) {
+            foreach ($matches['className'] as $i => $classNameMatch) {
+                $className = $classNameMatch[0];
+                // Offset of the full O:... pattern
+                $matchOffset = (int)$matches[0][$i][1];
+                $declaredLength = (int)$matches['length'][$i][0];
+
+                // Validate: 1) length matches, 2) not inside a string value
+                if (strlen($className) === $declaredLength && !$this->isInsideString($payload, $matchOffset)) {
+                    $classNames[] = $className;
+                }
+            }
+        }
+        return $classNames;
+    }
+
+    private function isInsideString(string $payload, int $offset): bool
+    {
+        if (preg_match_all('/s:(\d+):"/', $payload, $stringMatches, PREG_OFFSET_CAPTURE)) {
+            foreach ($stringMatches[0] as $i => $match) {
+                $stringDefOffset = $match[1];
+                $stringLength = (int)$stringMatches[1][$i][0];
+                // String content starts after s:LENGTH:"
+                $contentStart = $stringDefOffset + strlen($match[0]);
+                $contentEnd = $contentStart + $stringLength;
+
+                if ($offset >= $contentStart && $offset < $contentEnd) {
+                    return true;
+                }
+            }
+        }
+        return false;
+    }
+
+    /**
+     * @param list<class-string> $allowedClassNames
+     */
+    private function isInstanceOf(string $className, array $allowedClassNames): bool
+    {
+        foreach ($allowedClassNames as $allowedClassName) {
+            if (is_a($className, $allowedClassName, true) || is_subclass_of($className, $allowedClassName)) {
+                return true;
+            }
+        }
+        return false;
+    }
+}
diff --git a/typo3/sysext/core/Tests/Functional/Serializer/Fixtures/BoE4HIpmXv.message b/typo3/sysext/core/Tests/Functional/Serializer/Fixtures/BoE4HIpmXv.message
diff --git a/typo3/sysext/core/Tests/Functional/Serializer/PolymorphicDeserializerTest.php b/typo3/sysext/core/Tests/Functional/Serializer/PolymorphicDeserializerTest.php
@@ -0,0 +1,124 @@
+<?php
+
+declare(strict_types=1);
+
+/*
+ * This file is part of the TYPO3 CMS project.
+ *
+ * It is free software; you can redistribute it and/or modify it under
+ * the terms of the GNU General Public License, either version 2
+ * of the License, or any later version.
+ *
+ * For the full copyright and license information, please read the
+ * LICENSE.txt file that was distributed with this source code.
+ *
+ * The TYPO3 project - inspiring people to share!
+ */
+
+namespace TYPO3\CMS\Core\Tests\Functional\Serializer;
+
+use PHPUnit\Framework\Attributes\DataProvider;
+use PHPUnit\Framework\Attributes\Test;
+use Symfony\Component\Mailer\Envelope;
+use Symfony\Component\Mailer\SentMessage;
+use Symfony\Component\Mime\Address;
+use Symfony\Component\Mime\Header\HeaderInterface;
+use Symfony\Component\Mime\Header\Headers;
+use Symfony\Component\Mime\RawMessage;
+use TYPO3\CMS\Core\Serializer\Exception\PolymorphicDeserializerException;
+use TYPO3\CMS\Core\Serializer\PolymorphicDeserializer;
+use TYPO3\TestingFramework\Core\Functional\FunctionalTestCase;
+
+final class PolymorphicDeserializerTest extends FunctionalTestCase
+{
+    protected bool $initializeDatabase = false;
+
+    private PolymorphicDeserializer $subject;
+
+    protected function setUp(): void
+    {
+        parent::setUp();
+        $this->subject = new PolymorphicDeserializer();
+    }
+
+    #[Test]
+    public function spooledMailMessageCanBeDeserialized(): void
+    {
+        $payload = file_get_contents(__DIR__ . '/Fixtures/BoE4HIpmXv.message');
+        $result = $this->subject->deserialize($payload, [
+            SentMessage::class,
+            RawMessage::class,
+            Envelope::class,
+            Address::class,
+            Headers::class,
+            HeaderInterface::class,
+        ]);
+        self::assertInstanceOf(SentMessage::class, $result);
+    }
+
+    public static function spooledMailMessageCannotBeDeserializedDataProvider(): \Generator
+    {
+        yield [
+            file_get_contents(__DIR__ . '/Fixtures/BoE4HIpmXv.message'),
+            'Invalid class name "TYPO3\CMS\Core\Mail\FluidEmail" found in payload',
+            1767987405,
+        ];
+        yield [
+            's:foo:broken',
+            'Syntax error in payload, unable to de-serialize: unserialize(): Error at offset 0 of 12 bytes',
+            1768212616,
+        ];
+    }
+
+    #[Test]
+    #[DataProvider('spooledMailMessageCannotBeDeserializedDataProvider')]
+    public function spooledMailMessageCannotBeDeserialized(string $payload, string $expectedExceptionMessage, int $expectedExceptionCode): void
+    {
+        $this->expectException(PolymorphicDeserializerException::class);
+        $this->expectExceptionMessage($expectedExceptionMessage);
+        $this->expectExceptionCode($expectedExceptionCode);
+
+        $result = $this->subject->deserialize($payload, [SentMessage::class]);
+        self::assertInstanceOf(SentMessage::class, $result);
+    }
+
+    public static function canParseClassNamesDataProvider(): iterable
+    {
+        yield 'simple example' => [
+            'a:2:{i:0;O:10:"ValidClass":0:{}i:1;s:21:" O:12:"InvalidClass":0:{} ";i:2;O:333:"IncorrectLengthClass":0:{}}',
+            ['ValidClass'],
+        ];
+        yield 'serialized mail message' => [
+            file_get_contents(__DIR__ . '/Fixtures/BoE4HIpmXv.message'),
+            [
+                \Symfony\Component\Mailer\SentMessage::class,
+                \TYPO3\CMS\Core\Mail\FluidEmail::class,
+                \Symfony\Component\Mime\Header\Headers::class,
+                \Symfony\Component\Mime\Header\MailboxListHeader::class,
+                \Symfony\Component\Mime\Address::class,
+                \Symfony\Component\Mime\Header\MailboxListHeader::class,
+                \Symfony\Component\Mime\Address::class,
+                \Symfony\Component\Mime\Header\UnstructuredHeader::class,
+                \Symfony\Component\Mime\Header\UnstructuredHeader::class,
+                \Symfony\Component\Mime\RawMessage::class,
+                \Symfony\Component\Mailer\DelayedEnvelope::class,
+            ],
+        ];
+    }
+
+    #[Test]
+    #[DataProvider('canParseClassNamesDataProvider')]
+    public function canParseClassNames(string $payload, array $expectedClassNames): void
+    {
+        self::assertEquals($expectedClassNames, $this->subject->parseClassNames($payload));
+    }
+
+    #[Test]
+    public function falseValueCanBeDeserialized(): void
+    {
+        $payload = 'b:0;';
+        $result = $this->subject->deserialize($payload, []);
+
+        self::assertFalse($result);
+    }
+}
diff --git a/typo3/sysext/core/Tests/Unit/Mail/FileSpoolTest.php b/typo3/sysext/core/Tests/Unit/Mail/FileSpoolTest.php
