diff --git a/src/PdfParser/Type/PdfType.php b/src/PdfParser/Type/PdfType.php index 5330a44..b32eb44 100644 --- a/src/PdfParser/Type/PdfType.php +++ b/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; diff --git a/src/PdfReader/PdfReader.php b/src/PdfReader/PdfReader.php index f3effe3..d55de61 100644 --- a/src/PdfReader/PdfReader.php +++ b/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'); diff --git a/tests/functional/PdfParser/Type/PdfIndirectObjectTest.php b/tests/functional/PdfParser/Type/PdfIndirectObjectTest.php index 5c2f634..4314212 100644 --- a/tests/functional/PdfParser/Type/PdfIndirectObjectTest.php +++ b/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); } diff --git a/tests/functional/PdfReader/PdfReaderTest.php b/tests/functional/PdfReader/PdfReaderTest.php index cc189da..fe6e469 100644 --- a/tests/functional/PdfReader/PdfReaderTest.php +++ b/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 diff --git a/tests/unit/PdfParser/Type/PdfTypeTest.php b/tests/unit/PdfParser/Type/PdfTypeTest.php index 14de096..69a771d 100644 --- a/tests/unit/PdfParser/Type/PdfTypeTest.php +++ b/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); + } } \ No newline at end of file diff --git a/tests/unit/PdfReader/PdfReaderTest.php b/tests/unit/PdfReader/PdfReaderTest.php new file mode 100644 index 0000000..4b1f888 --- /dev/null +++ b/tests/unit/PdfReader/PdfReaderTest.php @@ -0,0 +1,99 @@ +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); + } +}