From 80503c483783f94a811e64e4ad1d8bfd6b10b94e Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lu=C3=ADs=20Cobucci?= <lcobucci@gmail.com>
Date: Tue, 13 Aug 2019 22:25:05 +0200
Subject: [PATCH] Guard cache regions against corrupted data

For some bizarre reason the underlying cache drivers are returning
unexpected values, which are leaking to the cache objects and causing
them to error.

This makes our cache regions much more strict about the types that are
fetched from the cache provider, ensuring that no invalid information is
ever sent to the hydrators.
---
 .../Cache/Region/DefaultMultiGetRegion.php    |  6 +++++
 .../ORM/Cache/Region/DefaultRegion.php        | 10 +++++--
 .../Tests/ORM/Cache/DefaultRegionTest.php     | 26 +++++++++++++++++++
 .../Tests/ORM/Cache/MultiGetRegionTest.php    | 13 ++++++++++
 4 files changed, 53 insertions(+), 2 deletions(-)

diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php
index 2cd94292b4..7b5f28ecf3 100644
--- a/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php
+++ b/lib/Doctrine/ORM/Cache/Region/DefaultMultiGetRegion.php
@@ -21,6 +21,7 @@
 namespace Doctrine\ORM\Cache\Region;
 
 use Doctrine\Common\Cache\MultiGetCache;
+use Doctrine\ORM\Cache\CacheEntry;
 use Doctrine\ORM\Cache\CollectionCacheEntry;
 
 /**
@@ -67,7 +68,12 @@ public function getMultiple(CollectionCacheEntry $collection)
         }
 
         $returnableItems = [];
+
         foreach ($keysToRetrieve as $index => $key) {
+            if (! $items[$key] instanceof CacheEntry) {
+                return null;
+            }
+
             $returnableItems[$index] = $items[$key];
         }
 
diff --git a/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php b/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php
index be62a6fb95..ab069cefb5 100644
--- a/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php
+++ b/lib/Doctrine/ORM/Cache/Region/DefaultRegion.php
@@ -94,7 +94,13 @@ public function contains(CacheKey $key)
      */
     public function get(CacheKey $key)
     {
-        return $this->cache->fetch($this->getCacheEntryKey($key)) ?: null;
+        $entry = $this->cache->fetch($this->getCacheEntryKey($key));
+
+        if (! $entry instanceof CacheEntry) {
+            return null;
+        }
+
+        return $entry;
     }
 
     /**
@@ -108,7 +114,7 @@ public function getMultiple(CollectionCacheEntry $collection)
             $entryKey   = $this->getCacheEntryKey($key);
             $entryValue = $this->cache->fetch($entryKey);
 
-            if ($entryValue === false) {
+            if (! $entryValue instanceof CacheEntry) {
                 return null;
             }
 
diff --git a/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php
index 97365dfcf2..e50c2a55c3 100644
--- a/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php
+++ b/tests/Doctrine/Tests/ORM/Cache/DefaultRegionTest.php
@@ -95,6 +95,32 @@ public function testGetMulti()
         $this->assertEquals($value1, $actual[0]);
         $this->assertEquals($value2, $actual[1]);
     }
+
+    /**
+     * @test
+     * @group GH7266
+     */
+    public function corruptedDataDoesNotLeakIntoApplicationWhenGettingSingleEntry() : void
+    {
+        $key1 = new CacheKeyMock('key.1');
+        $this->cache->save($this->region->getName() . '_' . $key1->hash, 'a-very-invalid-value');
+
+        self::assertTrue($this->region->contains($key1));
+        self::assertNull($this->region->get($key1));
+    }
+
+    /**
+     * @test
+     * @group GH7266
+     */
+    public function corruptedDataDoesNotLeakIntoApplicationWhenGettingMultipleEntries() : void
+    {
+        $key1 = new CacheKeyMock('key.1');
+        $this->cache->save($this->region->getName() . '_' . $key1->hash, 'a-very-invalid-value');
+
+        self::assertTrue($this->region->contains($key1));
+        self::assertNull($this->region->getMultiple(new CollectionCacheEntry([$key1])));
+    }
 }
 
 /**
diff --git a/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php
index 0d98665d0d..5f45de4714 100644
--- a/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php
+++ b/tests/Doctrine/Tests/ORM/Cache/MultiGetRegionTest.php
@@ -39,4 +39,17 @@ public function testGetMulti()
         $this->assertEquals($value1, $actual[0]);
         $this->assertEquals($value2, $actual[1]);
     }
+
+    /**
+     * @test
+     * @group GH7266
+     */
+    public function corruptedDataDoesNotLeakIntoApplication() : void
+    {
+        $key1 = new CacheKeyMock('key.1');
+        $this->cache->save($this->region->getName() . '_' . $key1->hash, 'a-very-invalid-value');
+
+        self::assertTrue($this->region->contains($key1));
+        self::assertNull($this->region->getMultiple(new CollectionCacheEntry([$key1])));
+    }
 }