Skip to content

Commit 701b518

Browse files
committed
Detect recursion loops resolving objects (fixes #51)
During parsing of an object, sometimes parts of the object have to be resolved. An example is stream lengths. If such an object directly or indirectly points to the object being parsed, it can cause an infinite loop. Guard against all cases of re-entrant resolution of objects.
1 parent afe0242 commit 701b518

File tree

7 files changed

+49
-1
lines changed

7 files changed

+49
-1
lines changed

Diff for: ChangeLog

+5
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,10 @@
11
2017-07-26 Jay Berkenbilt <ejb@ql.org>
22

3+
* Detect infinite loops while resolving objects. This could happen
4+
if something inside an object that had to be resolved during
5+
parsing, such as a stream length, recursively referenced the
6+
object being resolved.
7+
38
* CVE-2017-9208: Handle references to and appearance of object 0
49
as a special case. Object 0 is not allowed, and qpdf was using it
510
internally to represent direct objects.

Diff for: include/qpdf/QPDF.hh

+20
Original file line numberDiff line numberDiff line change
@@ -603,6 +603,25 @@ class QPDF
603603
int gen;
604604
};
605605

606+
class ResolveRecorder
607+
{
608+
public:
609+
ResolveRecorder(QPDF* qpdf, QPDFObjGen const& og) :
610+
qpdf(qpdf),
611+
og(og)
612+
{
613+
qpdf->resolving.insert(og);
614+
}
615+
virtual ~ResolveRecorder()
616+
{
617+
this->qpdf->resolving.erase(og);
618+
}
619+
private:
620+
QPDF* qpdf;
621+
QPDFObjGen og;
622+
};
623+
friend class ResolveRecorder;
624+
606625
void parse(char const* password);
607626
void warn(QPDFExc const& e);
608627
void setTrailer(QPDFObjectHandle obj);
@@ -1065,6 +1084,7 @@ class QPDF
10651084
std::map<QPDFObjGen, QPDFXRefEntry> xref_table;
10661085
std::set<int> deleted_objects;
10671086
std::map<QPDFObjGen, ObjCache> obj_cache;
1087+
std::set<QPDFObjGen> resolving;
10681088
QPDFObjectHandle trailer;
10691089
std::vector<QPDFObjectHandle> all_pages;
10701090
std::map<QPDFObjGen, int> pageobj_to_pages_pos;

Diff for: libqpdf/QPDF.cc

+15
Original file line numberDiff line numberDiff line change
@@ -1471,6 +1471,21 @@ QPDF::resolve(int objid, int generation)
14711471
// to insert things into the object cache that don't actually
14721472
// exist in the file.
14731473
QPDFObjGen og(objid, generation);
1474+
if (this->resolving.count(og))
1475+
{
1476+
// This can happen if an object references itself directly or
1477+
// indirectly in some key that has to be resolved during
1478+
// object parsing, such as stream length.
1479+
QTC::TC("qpdf", "QPDF recursion loop in resolve");
1480+
warn(QPDFExc(qpdf_e_damaged_pdf, this->file->getName(),
1481+
"", this->file->getLastOffset(),
1482+
"loop detected resolving object " +
1483+
QUtil::int_to_string(objid) + " " +
1484+
QUtil::int_to_string(generation)));
1485+
return new QPDF_Null;
1486+
}
1487+
ResolveRecorder rr(this, og);
1488+
14741489
if (! this->obj_cache.count(og))
14751490
{
14761491
if (! this->xref_table.count(og))

Diff for: qpdf/qpdf.testcov

+1
Original file line numberDiff line numberDiff line change
@@ -276,3 +276,4 @@ qpdf-c called qpdf_set_deterministic_ID 0
276276
QPDFObjectHandle indirect with 0 objid 0
277277
QPDF object id 0 0
278278
QPDF caught recursive xref reconstruction 0
279+
QPDF recursion loop in resolve 0

Diff for: qpdf/qtest/qpdf.test

+2-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ $td->runtest("remove page we don't have",
206206
show_ntests();
207207
# ----------
208208
$td->notify("--- Miscellaneous Tests ---");
209-
$n_tests += 81;
209+
$n_tests += 82;
210210

211211
$td->runtest("qpdf version",
212212
{$td->COMMAND => "qpdf --version"},
@@ -220,6 +220,7 @@ $td->runtest("C API: qpdf version",
220220

221221
# Files to reproduce various bugs
222222
foreach my $d (
223+
["51", "resolve loop"],
223224
["99", "object 0"],
224225
["99b", "object 0"],
225226
["100","xref reconstruction loop"],

Diff for: qpdf/qtest/qpdf/issue-51.out

+6
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,6 @@
1+
WARNING: issue-51.pdf: reported number of objects (0) inconsistent with actual number of objects (9)
2+
WARNING: issue-51.pdf (object 7 0, file position 553): expected endobj
3+
WARNING: issue-51.pdf (object 1 0, file position 359): expected endobj
4+
WARNING: issue-51.pdf (file position 70): loop detected resolving object 2 0
5+
WARNING: issue-51.pdf (object 2 0, file position 71): attempting to recover stream length
6+
issue-51.pdf (object 2 0, file position 71): unable to recover stream data

Diff for: qpdf/qtest/qpdf/issue-51.pdf

977 Bytes
Binary file not shown.

0 commit comments

Comments
 (0)