From 701b518d5c56a1449825a3a37a716c58e05e1c3e Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Wed, 26 Jul 2017 05:03:38 -0400 Subject: [PATCH] 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. --- ChangeLog | 5 +++++ include/qpdf/QPDF.hh | 20 ++++++++++++++++++++ libqpdf/QPDF.cc | 15 +++++++++++++++ qpdf/qpdf.testcov | 1 + qpdf/qtest/qpdf.test | 3 ++- qpdf/qtest/qpdf/issue-51.out | 6 ++++++ qpdf/qtest/qpdf/issue-51.pdf | 22 ++++++++++++++++++++++ 7 files changed, 71 insertions(+), 1 deletion(-) create mode 100644 qpdf/qtest/qpdf/issue-51.out create mode 100644 qpdf/qtest/qpdf/issue-51.pdf diff --git a/ChangeLog b/ChangeLog index f39c52c3..649e5aac 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,5 +1,10 @@ 2017-07-26 Jay Berkenbilt + * Detect infinite loops while resolving objects. This could happen + if something inside an object that had to be resolved during + parsing, such as a stream length, recursively referenced the + object being resolved. + * CVE-2017-9208: Handle references to and appearance of object 0 as a special case. Object 0 is not allowed, and qpdf was using it internally to represent direct objects. diff --git a/include/qpdf/QPDF.hh b/include/qpdf/QPDF.hh index f7a31edf..4742275f 100644 --- a/include/qpdf/QPDF.hh +++ b/include/qpdf/QPDF.hh @@ -603,6 +603,25 @@ class QPDF int gen; }; + class ResolveRecorder + { + public: + ResolveRecorder(QPDF* qpdf, QPDFObjGen const& og) : + qpdf(qpdf), + og(og) + { + qpdf->resolving.insert(og); + } + virtual ~ResolveRecorder() + { + this->qpdf->resolving.erase(og); + } + private: + QPDF* qpdf; + QPDFObjGen og; + }; + friend class ResolveRecorder; + void parse(char const* password); void warn(QPDFExc const& e); void setTrailer(QPDFObjectHandle obj); @@ -1065,6 +1084,7 @@ class QPDF std::map xref_table; std::set deleted_objects; std::map obj_cache; + std::set resolving; QPDFObjectHandle trailer; std::vector all_pages; std::map pageobj_to_pages_pos; diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index 846f188f..ecc13491 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -1471,6 +1471,21 @@ QPDF::resolve(int objid, int generation) // to insert things into the object cache that don't actually // exist in the file. QPDFObjGen og(objid, generation); + if (this->resolving.count(og)) + { + // This can happen if an object references itself directly or + // indirectly in some key that has to be resolved during + // object parsing, such as stream length. + QTC::TC("qpdf", "QPDF recursion loop in resolve"); + warn(QPDFExc(qpdf_e_damaged_pdf, this->file->getName(), + "", this->file->getLastOffset(), + "loop detected resolving object " + + QUtil::int_to_string(objid) + " " + + QUtil::int_to_string(generation))); + return new QPDF_Null; + } + ResolveRecorder rr(this, og); + if (! this->obj_cache.count(og)) { if (! this->xref_table.count(og)) diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index f3ddd60d..8144d2c3 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -276,3 +276,4 @@ qpdf-c called qpdf_set_deterministic_ID 0 QPDFObjectHandle indirect with 0 objid 0 QPDF object id 0 0 QPDF caught recursive xref reconstruction 0 +QPDF recursion loop in resolve 0 diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test index c45215fa..c0207019 100644 --- a/qpdf/qtest/qpdf.test +++ b/qpdf/qtest/qpdf.test @@ -206,7 +206,7 @@ $td->runtest("remove page we don't have", show_ntests(); # ---------- $td->notify("--- Miscellaneous Tests ---"); -$n_tests += 81; +$n_tests += 82; $td->runtest("qpdf version", {$td->COMMAND => "qpdf --version"}, @@ -220,6 +220,7 @@ $td->runtest("C API: qpdf version", # Files to reproduce various bugs foreach my $d ( + ["51", "resolve loop"], ["99", "object 0"], ["99b", "object 0"], ["100","xref reconstruction loop"], diff --git a/qpdf/qtest/qpdf/issue-51.out b/qpdf/qtest/qpdf/issue-51.out new file mode 100644 index 00000000..7f2192f6 --- /dev/null +++ b/qpdf/qtest/qpdf/issue-51.out @@ -0,0 +1,6 @@ +WARNING: issue-51.pdf: reported number of objects (0) inconsistent with actual number of objects (9) +WARNING: issue-51.pdf (object 7 0, file position 553): expected endobj +WARNING: issue-51.pdf (object 1 0, file position 359): expected endobj +WARNING: issue-51.pdf (file position 70): loop detected resolving object 2 0 +WARNING: issue-51.pdf (object 2 0, file position 71): attempting to recover stream length +issue-51.pdf (object 2 0, file position 71): unable to recover stream data diff --git a/qpdf/qtest/qpdf/issue-51.pdf b/qpdf/qtest/qpdf/issue-51.pdf new file mode 100644 index 00000000..2dafce1a --- /dev/null +++ b/qpdf/qtest/qpdf/issue-51.pdf @@ -0,0 +1,22 @@ +%PDF-100000000000002 0 obj +<> +stream +000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001 0 obj +<>/00000000 2 0 R>>000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007 0 obj +<>0000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000xref +0 9 +0000000000 00000 f +0000000200 00000 n +0000000009 00000 n +0000000000 00000 n +0000000000 00000 n +0000000000 00000 n +0000000000 00000 n +0000000400 00000 n +0000000000 00000 n +trailer<>startxref +740 +%%EOF \ No newline at end of file