From b9ace2c1bb05610b246b13853426cf05d715c100 Mon Sep 17 00:00:00 2001
From: Alexander Schnitzler <git@alexanderschnitzler.de>
Date: Thu, 14 May 2020 19:00:32 +0200
Subject: [PATCH] [BUGFIX] Properly (un)serialize ReflectionService

The ReflectionService usually doesn't get serialized by users
directly but since Extbase has an unclean dependency chain, the
serialization of the ReflectionService is triggered in user land
code when serializing a LazyObjectStorage e.g.

Since it's no problem to implement a clean serialization and
unserialization of the ReflectionService it is implemented with
this patch and will no longer cause any troubles.

There is just one thing to mention. The ReflectionService usually
comes with a cache which cannot be restored during wakeup of the
serialized service. It's unlikely but it's possible that the
absense of the cache can cause a performance hit.

Releases: master, 9.5
Resolves: #91404
Change-Id: I8c64968f0f329528c9f578ba0ef76437ada40ac0
Reviewed-on: https://review.typo3.org/c/Packages/TYPO3.CMS/+/64494
Tested-by: Susanne Moog <look@susi.dev>
Tested-by: Benjamin Franzke <bfr@qbus.de>
Tested-by: TYPO3com <noreply@typo3.com>
Reviewed-by: Susanne Moog <look@susi.dev>
Reviewed-by: Benjamin Franzke <bfr@qbus.de>
---
 .../Classes/Reflection/ReflectionService.php  | 44 ++++++++++----
 .../InsecureSerializedReflectionService.txt   |  1 +
 .../Unit/Reflection/ReflectionServiceTest.php | 57 +++++++++++++++++++
 3 files changed, 92 insertions(+), 10 deletions(-)
 create mode 100644 typo3/sysext/extbase/Tests/Unit/Reflection/Fixture/InsecureSerializedReflectionService.txt

diff --git a/typo3/sysext/extbase/Classes/Reflection/ReflectionService.php b/typo3/sysext/extbase/Classes/Reflection/ReflectionService.php
index 2117d096d22b..a15ba14dba61 100644
--- a/typo3/sysext/extbase/Classes/Reflection/ReflectionService.php
+++ b/typo3/sysext/extbase/Classes/Reflection/ReflectionService.php
@@ -16,9 +16,10 @@
 namespace TYPO3\CMS\Extbase\Reflection;
 
 use TYPO3\CMS\Core\Cache\CacheManager;
+use TYPO3\CMS\Core\Cache\Exception\NoSuchCacheException;
+use TYPO3\CMS\Core\Cache\Frontend\FrontendInterface;
 use TYPO3\CMS\Core\Core\Environment;
 use TYPO3\CMS\Core\Information\Typo3Version;
-use TYPO3\CMS\Core\Security\BlockSerializationTrait;
 use TYPO3\CMS\Core\SingletonInterface;
 use TYPO3\CMS\Extbase\Reflection\Exception\UnknownClassException;
 
@@ -28,15 +29,13 @@ use TYPO3\CMS\Extbase\Reflection\Exception\UnknownClassException;
  */
 class ReflectionService implements SingletonInterface
 {
-    use BlockSerializationTrait;
-
     /**
      * @var string
      */
     private static $cacheEntryIdentifier;
 
     /**
-     * @var \TYPO3\CMS\Core\Cache\Frontend\VariableFrontend
+     * @var FrontendInterface
      */
     protected $dataCache;
 
@@ -70,13 +69,19 @@ class ReflectionService implements SingletonInterface
      */
     public function __construct(CacheManager $cacheManager = null)
     {
-        if ($cacheManager instanceof CacheManager && $cacheManager->hasCache('extbase')) {
-            $this->cachingEnabled = true;
-            $this->dataCache = $cacheManager->getCache('extbase');
+        if ($cacheManager instanceof CacheManager) {
+            try {
+                $this->dataCache = $cacheManager->getCache('extbase');
+                $this->cachingEnabled = true;
+            } catch (NoSuchCacheException $ignoredException) {
+                $this->cachingEnabled = false;
+            }
 
-            static::$cacheEntryIdentifier = 'ClassSchemata_' . sha1((string)(new Typo3Version()) . Environment::getProjectPath());
-            if (($classSchemata = $this->dataCache->get(static::$cacheEntryIdentifier)) !== false) {
-                $this->classSchemata = $classSchemata;
+            if ($this->cachingEnabled) {
+                static::$cacheEntryIdentifier = 'ClassSchemata_' . sha1((string)(new Typo3Version()) . Environment::getProjectPath());
+                if (($classSchemata = $this->dataCache->get(static::$cacheEntryIdentifier)) !== false) {
+                    $this->classSchemata = $classSchemata;
+                }
             }
         }
     }
@@ -123,4 +128,23 @@ class ReflectionService implements SingletonInterface
         $this->dataCacheNeedsUpdate = true;
         return $classSchema;
     }
+
+    /**
+     * @internal
+     */
+    public function __sleep(): array
+    {
+        return [];
+    }
+
+    /**
+     * @internal
+     */
+    public function __wakeup(): void
+    {
+        $this->dataCache = null;
+        $this->dataCacheNeedsUpdate = false;
+        $this->classSchemata = [];
+        $this->cachingEnabled = false;
+    }
 }
diff --git a/typo3/sysext/extbase/Tests/Unit/Reflection/Fixture/InsecureSerializedReflectionService.txt b/typo3/sysext/extbase/Tests/Unit/Reflection/Fixture/InsecureSerializedReflectionService.txt
new file mode 100644
index 000000000000..1da5c4d416de
--- /dev/null
+++ b/typo3/sysext/extbase/Tests/Unit/Reflection/Fixture/InsecureSerializedReflectionService.txt
@@ -0,0 +1 @@
+O:46:"TYPO3\CMS\Extbase\Reflection\ReflectionService":4:{s:12:" * dataCache";N;s:23:" * dataCacheNeedsUpdate";b:0;s:16:" * classSchemata";a:1:{s:9:"className";s:31:"possibly malicious class schema";}s:62:" TYPO3\CMS\Extbase\Reflection\ReflectionService cachingEnabled";b:0;}
diff --git a/typo3/sysext/extbase/Tests/Unit/Reflection/ReflectionServiceTest.php b/typo3/sysext/extbase/Tests/Unit/Reflection/ReflectionServiceTest.php
index 14c87feb77cf..14ab6a0d82e0 100644
--- a/typo3/sysext/extbase/Tests/Unit/Reflection/ReflectionServiceTest.php
+++ b/typo3/sysext/extbase/Tests/Unit/Reflection/ReflectionServiceTest.php
@@ -17,6 +17,9 @@ declare(strict_types=1);
 
 namespace TYPO3\CMS\Extbase\Tests\Unit\Reflection;
 
+use TYPO3\CMS\Core\Cache\CacheManager;
+use TYPO3\CMS\Core\Cache\Frontend\NullFrontend;
+use TYPO3\CMS\Extbase\Reflection\ClassSchema;
 use TYPO3\CMS\Extbase\Reflection\Exception\UnknownClassException;
 use TYPO3\CMS\Extbase\Reflection\ReflectionService;
 use TYPO3\CMS\Extbase\Tests\Unit\Reflection\Fixture\DummyClassWithInvalidTypeHint;
@@ -53,4 +56,58 @@ class ReflectionServiceTest extends UnitTestCase
         $reflectionService = new ReflectionService();
         $reflectionService->getClassSchema(DummyClassWithInvalidTypeHint::class);
     }
+
+    /**
+     * @test
+     */
+    public function reflectionServiceCanBeSerializedAndUnserialized(): void
+    {
+        $class = new class() {
+        };
+
+        $reflectionService = new ReflectionService();
+        $serialized = serialize($reflectionService);
+        unset($reflectionService);
+
+        $reflectionService = unserialize($serialized, ['allowed_classes' => [ReflectionService::class]]);
+
+        self::assertInstanceOf(ReflectionService::class, $reflectionService);
+        self::assertInstanceOf(ClassSchema::class, $reflectionService->getClassSchema($class));
+    }
+
+    /**
+     * @test
+     */
+    public function reflectionServiceCanBeSerializedAndUnserializedWithCacheManager(): void
+    {
+        $cacheManager = $this->prophesize(CacheManager::class);
+        $cacheManager->getCache('extbase')->willReturn(new NullFrontend('extbase'));
+
+        $class = new class() {
+        };
+
+        $reflectionService = new ReflectionService($cacheManager->reveal());
+        $serialized = serialize($reflectionService);
+        unset($reflectionService);
+
+        $reflectionService = unserialize($serialized, ['allowed_classes' => [ReflectionService::class]]);
+
+        self::assertInstanceOf(ReflectionService::class, $reflectionService);
+        self::assertInstanceOf(ClassSchema::class, $reflectionService->getClassSchema($class));
+    }
+
+    /**
+     * @test
+     */
+    public function reflectionServiceIsResetDuringWakeUp(): void
+    {
+        $insecureString = file_get_contents(__DIR__ . '/Fixture/InsecureSerializedReflectionService.txt');
+        $reflectionService = unserialize($insecureString);
+
+        $reflectionClass = new \ReflectionClass($reflectionService);
+        $classSchemaProperty = $reflectionClass->getProperty('classSchemata');
+        $classSchemaProperty->setAccessible(true);
+
+        self::assertSame([], $classSchemaProperty->getValue($reflectionService));
+    }
 }
-- 
GitLab