CVE-2015-9252: Resource Management Errors (CWE-399)
value: An issue was discovered in QPDF before 7.0.0. Endless recursion causes stack exhaustion in QPDFTokenizer::resolveLiteral() in QPDFTokenizer.cc, related to the QPDF::resolve function in QPDF.cc.
CWE description: Weaknesses in this category are related to improper management of system resources.
CVSS v3 base score: 5.5
Published: 2018-02-13T19:29Z
Repository: https://github.com/qpdf/qpdf
Language: Other
Files changed: ChangeLog, include/qpdf/QPDF.hh, libqpdf/QPDF.cc, qpdf/qpdf.testcov, qpdf/qtest/qpdf.test, qpdf/qtest/qpdf/issue-51.out, qpdf/qtest/qpdf/issue-51.pdf
Commit date: 2017-07-26 06:24:07 -0400
Commit message: 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.

Diff: diff --git a/ChangeLog b/ChangeLog
index f39c52c..649e5aa 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,7 +1,12 @@
 2017-07-26  Jay Berkenbilt  <ejb@ql.org>
 
+	* 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.
 
 	* CVE-2017-9209: Fix infinite loop caused by attempting to
diff --git a/include/qpdf/QPDF.hh b/include/qpdf/QPDF.hh
index f7a31ed..4742275 100644
--- a/include/qpdf/QPDF.hh
+++ b/include/qpdf/QPDF.hh
@@ -601,10 +601,29 @@ class QPDF
         QPDF* qpdf;
         int objid;
         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);
     void read_xref(qpdf_offset_t offset);
     void reconstruct_xref(QPDFExc& e);
@@ -1063,10 +1082,11 @@ class QPDF
     int cached_key_generation;
     std::string pdf_version;
     std::map<QPDFObjGen, QPDFXRefEntry> xref_table;
     std::set<int> deleted_objects;
     std::map<QPDFObjGen, ObjCache> obj_cache;
+    std::set<QPDFObjGen> resolving;
     QPDFObjectHandle trailer;
     std::vector<QPDFObjectHandle> all_pages;
     std::map<QPDFObjGen, int> pageobj_to_pages_pos;
     bool pushed_inherited_attributes_to_pages;
     std::vector<QPDFExc> warnings;
diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc
index 846f188..ecc1349 100644
--- a/libqpdf/QPDF.cc
+++ b/libqpdf/QPDF.cc
@@ -1469,10 +1469,25 @@ QPDF::resolve(int objid, int generation)
 {
     // Check object cache before checking xref table.  This allows us
     // 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))
 	{
 	    // PDF spec says unknown objects resolve to the null object.
diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov
index f3ddd60..8144d2c 100644
--- a/qpdf/qpdf.testcov
+++ b/qpdf/qpdf.testcov
@@ -274,5 +274,6 @@ QPDFWriter linearized deterministic ID 1
 QPDFWriter deterministic with no data 0
 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 c45215f..c020701 100644
--- a/qpdf/qtest/qpdf.test
+++ b/qpdf/qtest/qpdf.test
@@ -204,11 +204,11 @@ $td->runtest("remove page we don't have",
 	     {$td->FILE => "page_api_1.out2", $td->EXIT_STATUS => 2},
 	     $td->NORMALIZE_NEWLINES);
 show_ntests();
 # ----------
 $td->notify("--- Miscellaneous Tests ---");
-$n_tests += 81;
+$n_tests += 82;
 
 $td->runtest("qpdf version",
 	     {$td->COMMAND => "qpdf --version"},
 	     {$td->REGEXP => "qpdf version \\S+\n.*", $td->EXIT_STATUS => 0},
 	     $td->NORMALIZE_NEWLINES);
@@ -218,10 +218,11 @@ $td->runtest("C API: qpdf version",
 	      $td->EXIT_STATUS => 0},
 	     $td->NORMALIZE_NEWLINES);
 
 # Files to reproduce various bugs
 foreach my $d (
+    ["51", "resolve loop"],
     ["99", "object 0"],
     ["99b", "object 0"],
     ["100","xref reconstruction loop"],
     ["101", "resolve for exception text"],
     )
diff --git a/qpdf/qtest/qpdf/issue-51.out b/qpdf/qtest/qpdf/issue-51.out
new file mode 100644
index 0000000..7f2192f
--- /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 0000000..2dafce1
--- /dev/null
+++ b/qpdf/qtest/qpdf/issue-51.pdf
@@ -0,0 +1,22 @@
+%PDF-100000000000002 0 obj
+<</Length 2 0 R/000000/00000000000>>
+stream
+000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001 0 obj
+<</0000000000000000 0 0 R/000000000 0 0 R/00000000[00000000000]/00000<</0/00000000000000000000000000000000>>/00000000 2 0 R>>000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007 0 obj
+<</0000/0000000/00000 0 0 R
+/0000000000[1 0 R 0000 null null 0]
+/0000(00000)
+>>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<</Size 0/Root 7 0 R>>startxref
+740
+%%EOF
\ No newline at end of file

Vulnerable code: $n_tests += 81;
Fixed code: 	* 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.

    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;

    std::set<QPDFObjGen> resolving;
    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);

QPDF recursion loop in resolve 0
$n_tests += 82;
    ["51", "resolve loop"],
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
%PDF-100000000000002 0 obj
<</Length 2 0 R/000000/00000000000>>
stream
000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000001 0 obj
<</0000000000000000 0 0 R/000000000 0 0 R/00000000[00000000000]/00000<</0/00000000000000000000000000000000>>/00000000 2 0 R>>000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000000007 0 obj
<</0000/0000000/00000 0 0 R
/0000000000[1 0 R 0000 null null 0]
/0000(00000)
>>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<</Size 0/Root 7 0 R>>startxref
740
%%EOF