CVE-2018-9918: Uncontrolled Recursion (CWE-674)
value: libqpdf.a in QPDF through 8.0.2 mishandles certain "expected dictionary key but found non-name object" cases, allowing remote attackers to cause a denial of service (stack exhaustion), related to the QPDFObjectHandle and QPDF_Dictionary classes, because nesting in direct objects is not restricted.
CWE description: The product does not properly control the amount of recursion that takes place,  consuming excessive resources, such as allocated memory or the program stack.
CVSS v3 base score: 7.8
Published: 2018-04-10T18:29Z
Repository: https://github.com/qpdf/qpdf
Language: Other
Files changed: ChangeLog, libqpdf/QPDFObjectHandle.cc, qpdf/qpdf.testcov, qpdf/qtest/qpdf.test, qpdf/qtest/qpdf/issue-146.out, qpdf/qtest/qpdf/issue-202.out
Commit date: 2018-04-15 16:11:22 -0400
Commit message: Limit depth of nesting in direct objects (fixes #202)

This fixes CVE-2018-9918.

Diff: diff --git a/ChangeLog b/ChangeLog
index 3a88135c..17c7a1c4 100644
--- a/ChangeLog
+++ b/ChangeLog
@@ -1,5 +1,10 @@
+2018-04-15  Jay Berkenbilt  <ejb@ql.org>
+
+	* Arbitrarily limit the depth of data structures represented by
+	direct object. This is CVE-2018-9918. Fixes #202.
+
 2018-03-06  Jay Berkenbilt  <ejb@ql.org>
 
 	* 8.0.2: release
 
 	* Properly handle pages with no contents. Fixes #194.
diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc
index c178a492..149668eb 100644
--- a/libqpdf/QPDFObjectHandle.cc
+++ b/libqpdf/QPDFObjectHandle.cc
@@ -1485,16 +1485,30 @@ QPDFObjectHandle::parseInternal(PointerHolder<InputSource> input,
 	    }
 	    break;
 
 	  case QPDFTokenizer::tt_array_open:
 	  case QPDFTokenizer::tt_dict_open:
-            olist_stack.push_back(std::vector<QPDFObjectHandle>());
-            state = st_start;
-            offset_stack.push_back(input->tell());
-            state_stack.push_back(
-                (token.getType() == QPDFTokenizer::tt_array_open) ?
-                st_array : st_dictionary);
+            if (olist_stack.size() > 500)
+            {
+		QTC::TC("qpdf", "QPDFObjectHandle too deep");
+                warn(context,
+                     QPDFExc(qpdf_e_damaged_pdf, input->getName(),
+                             object_description,
+                             input->getLastOffset(),
+                             "ignoring excessively deeply nested data structure"));
+                object = newNull();
+                state = st_top;
+            }
+            else
+            {
+                olist_stack.push_back(std::vector<QPDFObjectHandle>());
+                state = st_start;
+                offset_stack.push_back(input->tell());
+                state_stack.push_back(
+                    (token.getType() == QPDFTokenizer::tt_array_open) ?
+                    st_array : st_dictionary);
+            }
 	    break;
 
 	  case QPDFTokenizer::tt_bool:
 	    object = newBool((token.getValue() == "true"));
 	    break;
diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov
index 5374302f..3f055a86 100644
--- a/qpdf/qpdf.testcov
+++ b/qpdf/qpdf.testcov
@@ -333,5 +333,6 @@ QPDFObjectHandle dictionary ignoring removeKey 0
 QPDFObjectHandle dictionary ignoring removereplace 0
 QPDFObjectHandle numeric non-numeric 0
 QPDFObjectHandle erase array bounds 0
 qpdf-c called qpdf_check_pdf 0
 QPDF xref loop 0
+QPDFObjectHandle too deep 0
diff --git a/qpdf/qtest/qpdf.test b/qpdf/qtest/qpdf.test
index 92a9412b..b27b4c82 100644
--- a/qpdf/qtest/qpdf.test
+++ b/qpdf/qtest/qpdf.test
@@ -234,10 +234,11 @@ my @bug_tests = (
     ["146", "very deeply nested array", 2],
     ["147", "previously caused memory error", 2],
     ["148", "free memory on bad flate", 2],
     ["149", "xref prev pointer loop", 3],
     ["150", "integer overflow", 2],
+    ["202", "even more deeply nested dictionary", 2],
     );
 $n_tests += scalar(@bug_tests);
 foreach my $d (@bug_tests)
 {
     my ($n, $description, $exit_status) = @$d;
diff --git a/qpdf/qtest/qpdf/issue-146.out b/qpdf/qtest/qpdf/issue-146.out
index 79bb8118..a275957f 100644
--- a/qpdf/qtest/qpdf/issue-146.out
+++ b/qpdf/qtest/qpdf/issue-146.out
@@ -1,7 +1,5 @@
 WARNING: issue-146.pdf: file is damaged
 WARNING: issue-146.pdf: can't find startxref
 WARNING: issue-146.pdf: Attempting to reconstruct cross-reference table
-WARNING: issue-146.pdf (trailer, offset 20728): unknown token while reading object; treating as string
-WARNING: issue-146.pdf (trailer, offset 20732): unexpected EOF
-WARNING: issue-146.pdf (trailer, offset 20732): parse error while reading object
+WARNING: issue-146.pdf (trailer, offset 695): ignoring excessively deeply nested data structure
 issue-146.pdf: unable to find trailer dictionary while recovering damaged file
diff --git a/qpdf/qtest/qpdf/issue-202.out b/qpdf/qtest/qpdf/issue-202.out
new file mode 100644
index 00000000..edac1f2d
--- /dev/null
+++ b/qpdf/qtest/qpdf/issue-202.out
@@ -0,0 +1,5 @@
+WARNING: issue-202.pdf (trailer, offset 55770): ignoring excessively deeply nested data structure
+WARNING: issue-202.pdf: file is damaged
+WARNING: issue-202.pdf (offset 54769): expected trailer dictionary
+WARNING: issue-202.pdf: Attempting to reconstruct cross-reference table
+issue-202.pdf: unable to find trailer dictionary while recovering damaged file
diff --git a/qpdf/qtest/qpdf/issue-202.pdf b/qpdf/qtest/qpdf/issue-202.pdf
new file mode 100644
index 00000000..193d58a1
Binary files /dev/null and b/qpdf/qtest/qpdf/issue-202.pdf differ

Vulnerable code:             olist_stack.push_back(std::vector<QPDFObjectHandle>());
            state = st_start;
            offset_stack.push_back(input->tell());
            state_stack.push_back(
                (token.getType() == QPDFTokenizer::tt_array_open) ?
                st_array : st_dictionary);
WARNING: issue-146.pdf (trailer, offset 20728): unknown token while reading object; treating as string
WARNING: issue-146.pdf (trailer, offset 20732): unexpected EOF
WARNING: issue-146.pdf (trailer, offset 20732): parse error while reading object
Fixed code: 2018-04-15  Jay Berkenbilt  <ejb@ql.org>

	* Arbitrarily limit the depth of data structures represented by
	direct object. This is CVE-2018-9918. Fixes #202.

            if (olist_stack.size() > 500)
            {
		QTC::TC("qpdf", "QPDFObjectHandle too deep");
                warn(context,
                     QPDFExc(qpdf_e_damaged_pdf, input->getName(),
                             object_description,
                             input->getLastOffset(),
                             "ignoring excessively deeply nested data structure"));
                object = newNull();
                state = st_top;
            }
            else
            {
                olist_stack.push_back(std::vector<QPDFObjectHandle>());
                state = st_start;
                offset_stack.push_back(input->tell());
                state_stack.push_back(
                    (token.getType() == QPDFTokenizer::tt_array_open) ?
                    st_array : st_dictionary);
            }
QPDFObjectHandle too deep 0
    ["202", "even more deeply nested dictionary", 2],
WARNING: issue-146.pdf (trailer, offset 695): ignoring excessively deeply nested data structure
WARNING: issue-202.pdf (trailer, offset 55770): ignoring excessively deeply nested data structure
WARNING: issue-202.pdf: file is damaged
WARNING: issue-202.pdf (offset 54769): expected trailer dictionary
WARNING: issue-202.pdf: Attempting to reconstruct cross-reference table
issue-202.pdf: unable to find trailer dictionary while recovering damaged file