Fixed handling of recursive structures to avoid memory exhaustion

src/PdfParser/Type/PdfType.php

@@ -27,22 +27,33 @@ class PdfType
      * @param PdfType $value
      * @param PdfParser $parser
      * @param bool $stopAtIndirectObject
+     * @param array $ensuredObjectsList A list of all ensured indirect objects to prevent recursion
      * @return PdfType
      * @throws CrossReferenceException
      * @throws PdfParserException
      */
-    public static function resolve(PdfType $value, PdfParser $parser, $stopAtIndirectObject = false)
-    {
+    public static function resolve(
+        PdfType $value,
+        PdfParser $parser,
+        $stopAtIndirectObject = false,
+        array &$ensuredObjectsList = []
+    ) {
+        if ($value instanceof PdfIndirectObjectReference) {
+            $value = $parser->getIndirectObject($value->value);
+        }
+
         if ($value instanceof PdfIndirectObject) {
             if ($stopAtIndirectObject === true) {
                 return $value;
             }
 
-            return self::resolve($value->value, $parser, $stopAtIndirectObject);
-        }
-
-        if ($value instanceof PdfIndirectObjectReference) {
-            return self::resolve($parser->getIndirectObject($value->value), $parser, $stopAtIndirectObject);
+            if (\in_array($value->objectNumber, $ensuredObjectsList, true)) {
+                throw new PdfParserException(
+                    \sprintf('Indirect reference recursion detected (%s).', $value->objectNumber)
+                );
+            }
+            $ensuredObjectsList[] = $value->objectNumber;
+            return self::resolve($value->value, $parser, $stopAtIndirectObject, $ensuredObjectsList);
         }
 
         return $value;

src/PdfReader/PdfReader.php

@@ -139,12 +139,19 @@ public function getPage($pageNumber)
         $page = $this->pages[$pageNumber - 1];
 
         if ($page instanceof PdfIndirectObjectReference) {
-            $readPages = function ($kids) use (&$readPages) {
+            $alreadyReadKids = [];
+            $readPages = function ($kids) use (&$readPages, &$alreadyReadKids) {
                 $kids = PdfArray::ensure($kids);
 
                 /** @noinspection LoopWhichDoesNotLoopInspection */
                 foreach ($kids->value as $reference) {
                     $reference = PdfIndirectObjectReference::ensure($reference);
+
+                    if (\in_array($reference->value, $alreadyReadKids, true)) {
+                        throw new PdfReaderException('Recursive pages dictionary detected.');
+                    }
+                    $alreadyReadKids[] = $reference->value;
+
                     $object = $this->parser->getIndirectObject($reference->value);
                     $type = PdfDictionary::get($object->value, 'Type');
 
@@ -168,6 +175,7 @@ public function getPage($pageNumber)
             if ($type->value === 'Pages') {
                 $kids = PdfType::resolve(PdfDictionary::get($dict, 'Kids'), $this->parser);
                 try {
+                    $alreadyReadKids[] = $page->objectNumber;
                     $page = $this->pages[$pageNumber - 1] = $readPages($kids);
                 } catch (PdfReaderException $e) {
                     if ($e->getCode() !== PdfReaderException::KIDS_EMPTY) {
@@ -203,7 +211,8 @@ protected function readPages($readAll = false)
         }
 
         $expectedPageCount = $this->getPageCount();
-        $readPages = function ($kids, $count) use (&$readPages, $readAll, $expectedPageCount) {
+        $alreadyReadKids = [];
+        $readPages = function ($kids, $count) use (&$readPages, &$alreadyReadKids, $readAll, $expectedPageCount) {
             $kids = PdfArray::ensure($kids);
             $isLeaf = ($count->value === \count($kids->value));
 
@@ -215,6 +224,11 @@ protected function readPages($readAll = false)
                     continue;
                 }
 
+                if (\in_array($reference->value, $alreadyReadKids, true)) {
+                    throw new PdfReaderException('Recursive pages dictionary detected.');
+                }
+                $alreadyReadKids[] = $reference->value;
+
                 $object = $this->parser->getIndirectObject($reference->value);
                 $type = PdfDictionary::get($object->value, 'Type');
 

tests/functional/PdfParser/Type/PdfIndirectObjectTest.php

@@ -5,14 +5,10 @@
 use PHPUnit\Framework\TestCase;
 use setasign\Fpdi\PdfParser\PdfParser;
 use setasign\Fpdi\PdfParser\StreamReader;
-use setasign\Fpdi\PdfParser\Type\PdfBoolean;
 use setasign\Fpdi\PdfParser\Type\PdfDictionary;
 use setasign\Fpdi\PdfParser\Type\PdfIndirectObject;
-use setasign\Fpdi\PdfParser\Type\PdfIndirectObjectReference;
-use setasign\Fpdi\PdfParser\Type\PdfNull;
 use setasign\Fpdi\PdfParser\Type\PdfNumeric;
 use setasign\Fpdi\PdfParser\Type\PdfStream;
-use setasign\Fpdi\PdfParser\Type\PdfToken;
 
 class PdfIndirectObjectTest extends TestCase
 {
@@ -129,7 +125,6 @@ public function testParse($objectNumberToken, $generationNumberToken, $in, $expe
         if ($result->value instanceof PdfStream) {
             $this->assertEquals($expectedResult->value->value, $result->value->value);
             $this->assertSame($expectedResult->value->getStream(), $result->value->getStream());
-
         } else {
             $this->assertEquals($expectedResult, $result);
         }

tests/functional/PdfReader/PdfReaderTest.php

@@ -3,7 +3,6 @@
 namespace setasign\Fpdi\functional\PdfReader;
 
 use PHPUnit\Framework\TestCase;
-use Prophecy\Exception\InvalidArgumentException;
 use setasign\Fpdi\PdfParser\CrossReference\CrossReferenceException;
 use setasign\Fpdi\PdfParser\PdfParser;
 use setasign\Fpdi\PdfParser\StreamReader;
@@ -14,7 +13,6 @@
 use setasign\Fpdi\PdfParser\Type\PdfIndirectObjectReference;
 use setasign\Fpdi\PdfParser\Type\PdfName;
 use setasign\Fpdi\PdfParser\Type\PdfNumeric;
-use setasign\Fpdi\PdfParser\Type\PdfType;
 use setasign\Fpdi\PdfReader\PdfReader;
 
 class PdfReaderTest extends TestCase

tests/unit/PdfParser/Type/PdfTypeTest.php

@@ -4,6 +4,7 @@
 
 use PHPUnit\Framework\TestCase;
 use setasign\Fpdi\PdfParser\PdfParser;
+use setasign\Fpdi\PdfParser\PdfParserException;
 use setasign\Fpdi\PdfParser\Type\PdfIndirectObject;
 use setasign\Fpdi\PdfParser\Type\PdfIndirectObjectReference;
 use setasign\Fpdi\PdfParser\Type\PdfString;
@@ -118,4 +119,25 @@ public function testResolveWithMoreIndirectObjectReferenceAndStopParameter()
 
         $this->assertSame($indirectObject1, $result);
     }
+
+    public function testResolveWithRecursiveReferences()
+    {
+        $parser = (
+            $this->getMockBuilder(PdfParser::class)
+            ->setMethods(['getCatalog', 'getIndirectObject'])
+            ->disableOriginalConstructor()
+            ->getMock()
+        );
+
+        $object1 = PdfIndirectObject::create(1, 0, PdfIndirectObjectReference::create(2, 0));
+        $object2 = PdfIndirectObject::create(2, 0, PdfIndirectObjectReference::create(1, 0));
+        $parser->method('getIndirectObject')->willReturnMap([
+            [1, false, $object1],
+            [2, false, $object2],
+        ]);
+
+        $this->expectException(PdfParserException::class);
+        $this->expectExceptionMessage('Indirect reference recursion detected (1).');
+        PdfType::resolve($object1, $parser);
+    }
 }

tests/unit/PdfReader/PdfReaderTest.php

@@ -0,0 +1,99 @@
+<?php
+
+declare(strict_types=1);
+
+namespace setasign\Fpdi\unit\PdfReader;
+
+use PHPUnit\Framework\TestCase;
+use setasign\Fpdi\PdfParser\PdfParser;
+use setasign\Fpdi\PdfParser\Type\PdfArray;
+use setasign\Fpdi\PdfParser\Type\PdfDictionary;
+use setasign\Fpdi\PdfParser\Type\PdfIndirectObject;
+use setasign\Fpdi\PdfParser\Type\PdfIndirectObjectReference;
+use setasign\Fpdi\PdfParser\Type\PdfName;
+use setasign\Fpdi\PdfParser\Type\PdfNumeric;
+use setasign\Fpdi\PdfReader\PdfReader;
+use setasign\Fpdi\PdfReader\PdfReaderException;
+
+class PdfReaderTest extends TestCase
+{
+    public function testHandlingOfRecursivePageTreeStructure()
+    {
+        $parser = (
+            $this->getMockBuilder(PdfParser::class)
+            ->setMethods(['getCatalog', 'getIndirectObject'])
+            ->disableOriginalConstructor()
+            ->getMock()
+        );
+
+        $pages1 = PdfIndirectObject::create(2, 0, PdfDictionary::create([
+            'Type' => PdfName::create('Pages'),
+            'Count' => PdfNumeric::create(1),
+            'Kids' => PdfArray::create([PdfIndirectObjectReference::create(3, 0)])
+        ]));
+        $pages2 = PdfIndirectObject::create(3, 0, PdfDictionary::create([
+            'Type' => PdfName::create('Pages'),
+            'Parent' => PdfIndirectObjectReference::create(2, 0),
+            'Count' => PdfNumeric::create(1),
+            'Kids' => PdfArray::create([PdfIndirectObjectReference::create(2, 0)])
+        ]));
+        $parser->method('getIndirectObject')->willReturnMap([
+            [2, false, $pages1],
+            [3, false, $pages2],
+        ]);
+
+        $parser->method('getCatalog')->willReturn(PdfDictionary::create([
+            'Pages' => PdfDictionary::create([
+                'Count' => PdfNumeric::create(1),
+                'Kids' => PdfArray::create([PdfIndirectObjectReference::create(2, 0)])
+            ])
+        ]));
+
+        $pdfReader = new PdfReader($parser);
+        $this->assertEquals(1, $pdfReader->getPageCount());
+
+        $this->expectException(PdfReaderException::class);
+        $this->expectExceptionMessage('Recursive pages dictionary detected.');
+        $pdfReader->getPage(1);
+    }
+
+    public function testHandlingOfRecursivePageTreeStructureWhenFullTreeIsRead()
+    {
+        $parser = (
+            $this->getMockBuilder(PdfParser::class)
+            ->setMethods(['getCatalog', 'getIndirectObject'])
+            ->disableOriginalConstructor()
+            ->getMock()
+        );
+
+        $pages1 = PdfIndirectObject::create(2, 0, PdfDictionary::create([
+            'Type' => PdfName::create('Pages'),
+            'Count' => PdfNumeric::create(3),
+            'Kids' => PdfArray::create([PdfIndirectObjectReference::create(3, 0)])
+        ]));
+        $pages2 = PdfIndirectObject::create(3, 0, PdfDictionary::create([
+            'Type' => PdfName::create('Pages'),
+            'Parent' => PdfIndirectObjectReference::create(2, 0),
+            'Count' => PdfNumeric::create(2),
+            'Kids' => PdfArray::create([PdfIndirectObjectReference::create(2, 0)])
+        ]));
+        $parser->method('getIndirectObject')->willReturnMap([
+            [2, false, $pages1],
+            [3, false, $pages2],
+        ]);
+
+        $parser->method('getCatalog')->willReturn(PdfDictionary::create([
+            'Pages' => PdfDictionary::create([
+                'Count' => PdfNumeric::create(5),
+                'Kids' => PdfArray::create([PdfIndirectObjectReference::create(2, 0)])
+            ])
+        ]));
+
+        $pdfReader = new PdfReader($parser);
+        $this->assertEquals(5, $pdfReader->getPageCount());
+
+        $this->expectException(PdfReaderException::class);
+        $this->expectExceptionMessage('Recursive pages dictionary detected.');
+        $pdfReader->getPage(1);
+    }
+}