Skip to content

Commit ba671ba

Browse files
JanSlabonJanSlabon
authored
Merge commit from fork
Fixed infinite recursion in pages dictionary and in PdfType::resolve()
2 parents 4fc6d6e + 3a3b037 commit ba671ba

File tree

6 files changed

+155
-16
lines changed

6 files changed

+155
-16
lines changed

‎src/PdfParser/Type/PdfType.php‎

Lines changed: 18 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,22 +27,33 @@ class PdfType
2727
* @param PdfType $value
2828
* @param PdfParser $parser
2929
* @param bool $stopAtIndirectObject
30+
* @param array $ensuredObjectsList A list of all ensured indirect objects to prevent recursion
3031
* @return PdfType
3132
* @throws CrossReferenceException
3233
* @throws PdfParserException
3334
*/
34-
public static function resolve(PdfType $value, PdfParser $parser, $stopAtIndirectObject = false)
35-
{
35+
public static function resolve(
36+
PdfType $value,
37+
PdfParser $parser,
38+
$stopAtIndirectObject = false,
39+
array &$ensuredObjectsList = []
40+
) {
41+
if ($value instanceof PdfIndirectObjectReference) {
42+
$value = $parser->getIndirectObject($value->value);
43+
}
44+
3645
if ($value instanceof PdfIndirectObject) {
3746
if ($stopAtIndirectObject === true) {
3847
return $value;
3948
}
4049

41-
return self::resolve($value->value, $parser, $stopAtIndirectObject);
42-
}
43-
44-
if ($value instanceof PdfIndirectObjectReference) {
45-
return self::resolve($parser->getIndirectObject($value->value), $parser, $stopAtIndirectObject);
50+
if (\in_array($value->objectNumber, $ensuredObjectsList, true)) {
51+
throw new PdfParserException(
52+
\sprintf('Indirect reference recursion detected (%s).', $value->objectNumber)
53+
);
54+
}
55+
$ensuredObjectsList[] = $value->objectNumber;
56+
return self::resolve($value->value, $parser, $stopAtIndirectObject, $ensuredObjectsList);
4657
}
4758

4859
return $value;

‎src/PdfReader/PdfReader.php‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,12 +139,19 @@ public function getPage($pageNumber)
139139
$page = $this->pages[$pageNumber - 1];
140140

141141
if ($page instanceof PdfIndirectObjectReference) {
142-
$readPages = function ($kids) use (&$readPages) {
142+
$alreadyReadKids = [];
143+
$readPages = function ($kids) use (&$readPages, &$alreadyReadKids) {
143144
$kids = PdfArray::ensure($kids);
144145

145146
/** @noinspection LoopWhichDoesNotLoopInspection */
146147
foreach ($kids->value as $reference) {
147148
$reference = PdfIndirectObjectReference::ensure($reference);
149+
150+
if (\in_array($reference->value, $alreadyReadKids, true)) {
151+
throw new PdfReaderException('Recursive pages dictionary detected.');
152+
}
153+
$alreadyReadKids[] = $reference->value;
154+
148155
$object = $this->parser->getIndirectObject($reference->value);
149156
$type = PdfDictionary::get($object->value, 'Type');
150157

@@ -168,6 +175,7 @@ public function getPage($pageNumber)
168175
if ($type->value === 'Pages') {
169176
$kids = PdfType::resolve(PdfDictionary::get($dict, 'Kids'), $this->parser);
170177
try {
178+
$alreadyReadKids[] = $page->objectNumber;
171179
$page = $this->pages[$pageNumber - 1] = $readPages($kids);
172180
} catch (PdfReaderException $e) {
173181
if ($e->getCode() !== PdfReaderException::KIDS_EMPTY) {
@@ -203,7 +211,8 @@ protected function readPages($readAll = false)
203211
}
204212

205213
$expectedPageCount = $this->getPageCount();
206-
$readPages = function ($kids, $count) use (&$readPages, $readAll, $expectedPageCount) {
214+
$alreadyReadKids = [];
215+
$readPages = function ($kids, $count) use (&$readPages, &$alreadyReadKids, $readAll, $expectedPageCount) {
207216
$kids = PdfArray::ensure($kids);
208217
$isLeaf = ($count->value === \count($kids->value));
209218

@@ -215,6 +224,11 @@ protected function readPages($readAll = false)
215224
continue;
216225
}
217226

227+
if (\in_array($reference->value, $alreadyReadKids, true)) {
228+
throw new PdfReaderException('Recursive pages dictionary detected.');
229+
}
230+
$alreadyReadKids[] = $reference->value;
231+
218232
$object = $this->parser->getIndirectObject($reference->value);
219233
$type = PdfDictionary::get($object->value, 'Type');
220234

‎tests/functional/PdfParser/Type/PdfIndirectObjectTest.php‎

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -5,14 +5,10 @@
55
use PHPUnit\Framework\TestCase;
66
use setasign\Fpdi\PdfParser\PdfParser;
77
use setasign\Fpdi\PdfParser\StreamReader;
8-
use setasign\Fpdi\PdfParser\Type\PdfBoolean;
98
use setasign\Fpdi\PdfParser\Type\PdfDictionary;
109
use setasign\Fpdi\PdfParser\Type\PdfIndirectObject;
11-
use setasign\Fpdi\PdfParser\Type\PdfIndirectObjectReference;
12-
use setasign\Fpdi\PdfParser\Type\PdfNull;
1310
use setasign\Fpdi\PdfParser\Type\PdfNumeric;
1411
use setasign\Fpdi\PdfParser\Type\PdfStream;
15-
use setasign\Fpdi\PdfParser\Type\PdfToken;
1612

1713
class PdfIndirectObjectTest extends TestCase
1814
{
@@ -129,7 +125,6 @@ public function testParse($objectNumberToken, $generationNumberToken, $in, $expe
129125
if ($result->value instanceof PdfStream) {
130126
$this->assertEquals($expectedResult->value->value, $result->value->value);
131127
$this->assertSame($expectedResult->value->getStream(), $result->value->getStream());
132-
133128
} else {
134129
$this->assertEquals($expectedResult, $result);
135130
}

‎tests/functional/PdfReader/PdfReaderTest.php‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace setasign\Fpdi\functional\PdfReader;
44

55
use PHPUnit\Framework\TestCase;
6-
use Prophecy\Exception\InvalidArgumentException;
76
use setasign\Fpdi\PdfParser\CrossReference\CrossReferenceException;
87
use setasign\Fpdi\PdfParser\PdfParser;
98
use setasign\Fpdi\PdfParser\StreamReader;
@@ -14,7 +13,6 @@
1413
use setasign\Fpdi\PdfParser\Type\PdfIndirectObjectReference;
1514
use setasign\Fpdi\PdfParser\Type\PdfName;
1615
use setasign\Fpdi\PdfParser\Type\PdfNumeric;
17-
use setasign\Fpdi\PdfParser\Type\PdfType;
1816
use setasign\Fpdi\PdfReader\PdfReader;
1917

2018
class PdfReaderTest extends TestCase

‎tests/unit/PdfParser/Type/PdfTypeTest.php‎

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use PHPUnit\Framework\TestCase;
66
use setasign\Fpdi\PdfParser\PdfParser;
7+
use setasign\Fpdi\PdfParser\PdfParserException;
78
use setasign\Fpdi\PdfParser\Type\PdfIndirectObject;
89
use setasign\Fpdi\PdfParser\Type\PdfIndirectObjectReference;
910
use setasign\Fpdi\PdfParser\Type\PdfString;
@@ -118,4 +119,25 @@ public function testResolveWithMoreIndirectObjectReferenceAndStopParameter()
118119

119120
$this->assertSame($indirectObject1, $result);
120121
}
122+
123+
public function testResolveWithRecursiveReferences()
124+
{
125+
$parser = (
126+
$this->getMockBuilder(PdfParser::class)
127+
->setMethods(['getCatalog', 'getIndirectObject'])
128+
->disableOriginalConstructor()
129+
->getMock()
130+
);
131+
132+
$object1 = PdfIndirectObject::create(1, 0, PdfIndirectObjectReference::create(2, 0));
133+
$object2 = PdfIndirectObject::create(2, 0, PdfIndirectObjectReference::create(1, 0));
134+
$parser->method('getIndirectObject')->willReturnMap([
135+
[1, false, $object1],
136+
[2, false, $object2],
137+
]);
138+
139+
$this->expectException(PdfParserException::class);
140+
$this->expectExceptionMessage('Indirect reference recursion detected (1).');
141+
PdfType::resolve($object1, $parser);
142+
}
121143
}

‎tests/unit/PdfReader/PdfReaderTest.php‎

Lines changed: 99 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,99 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace setasign\Fpdi\unit\PdfReader;
6+
7+
use PHPUnit\Framework\TestCase;
8+
use setasign\Fpdi\PdfParser\PdfParser;
9+
use setasign\Fpdi\PdfParser\Type\PdfArray;
10+
use setasign\Fpdi\PdfParser\Type\PdfDictionary;
11+
use setasign\Fpdi\PdfParser\Type\PdfIndirectObject;
12+
use setasign\Fpdi\PdfParser\Type\PdfIndirectObjectReference;
13+
use setasign\Fpdi\PdfParser\Type\PdfName;
14+
use setasign\Fpdi\PdfParser\Type\PdfNumeric;
15+
use setasign\Fpdi\PdfReader\PdfReader;
16+
use setasign\Fpdi\PdfReader\PdfReaderException;
17+
18+
class PdfReaderTest extends TestCase
19+
{
20+
public function testHandlingOfRecursivePageTreeStructure()
21+
{
22+
$parser = (
23+
$this->getMockBuilder(PdfParser::class)
24+
->setMethods(['getCatalog', 'getIndirectObject'])
25+
->disableOriginalConstructor()
26+
->getMock()
27+
);
28+
29+
$pages1 = PdfIndirectObject::create(2, 0, PdfDictionary::create([
30+
'Type' => PdfName::create('Pages'),
31+
'Count' => PdfNumeric::create(1),
32+
'Kids' => PdfArray::create([PdfIndirectObjectReference::create(3, 0)])
33+
]));
34+
$pages2 = PdfIndirectObject::create(3, 0, PdfDictionary::create([
35+
'Type' => PdfName::create('Pages'),
36+
'Parent' => PdfIndirectObjectReference::create(2, 0),
37+
'Count' => PdfNumeric::create(1),
38+
'Kids' => PdfArray::create([PdfIndirectObjectReference::create(2, 0)])
39+
]));
40+
$parser->method('getIndirectObject')->willReturnMap([
41+
[2, false, $pages1],
42+
[3, false, $pages2],
43+
]);
44+
45+
$parser->method('getCatalog')->willReturn(PdfDictionary::create([
46+
'Pages' => PdfDictionary::create([
47+
'Count' => PdfNumeric::create(1),
48+
'Kids' => PdfArray::create([PdfIndirectObjectReference::create(2, 0)])
49+
])
50+
]));
51+
52+
$pdfReader = new PdfReader($parser);
53+
$this->assertEquals(1, $pdfReader->getPageCount());
54+
55+
$this->expectException(PdfReaderException::class);
56+
$this->expectExceptionMessage('Recursive pages dictionary detected.');
57+
$pdfReader->getPage(1);
58+
}
59+
60+
public function testHandlingOfRecursivePageTreeStructureWhenFullTreeIsRead()
61+
{
62+
$parser = (
63+
$this->getMockBuilder(PdfParser::class)
64+
->setMethods(['getCatalog', 'getIndirectObject'])
65+
->disableOriginalConstructor()
66+
->getMock()
67+
);
68+
69+
$pages1 = PdfIndirectObject::create(2, 0, PdfDictionary::create([
70+
'Type' => PdfName::create('Pages'),
71+
'Count' => PdfNumeric::create(3),
72+
'Kids' => PdfArray::create([PdfIndirectObjectReference::create(3, 0)])
73+
]));
74+
$pages2 = PdfIndirectObject::create(3, 0, PdfDictionary::create([
75+
'Type' => PdfName::create('Pages'),
76+
'Parent' => PdfIndirectObjectReference::create(2, 0),
77+
'Count' => PdfNumeric::create(2),
78+
'Kids' => PdfArray::create([PdfIndirectObjectReference::create(2, 0)])
79+
]));
80+
$parser->method('getIndirectObject')->willReturnMap([
81+
[2, false, $pages1],
82+
[3, false, $pages2],
83+
]);
84+
85+
$parser->method('getCatalog')->willReturn(PdfDictionary::create([
86+
'Pages' => PdfDictionary::create([
87+
'Count' => PdfNumeric::create(5),
88+
'Kids' => PdfArray::create([PdfIndirectObjectReference::create(2, 0)])
89+
])
90+
]));
91+
92+
$pdfReader = new PdfReader($parser);
93+
$this->assertEquals(5, $pdfReader->getPageCount());
94+
95+
$this->expectException(PdfReaderException::class);
96+
$this->expectExceptionMessage('Recursive pages dictionary detected.');
97+
$pdfReader->getPage(1);
98+
}
99+
}

0 commit comments

Comments
 (0)