Fix bugs found by fuzz tests

* Several assertions in linearization were not always true; change them to run time errors * Handle a few cases of uninitialized objects * Handle pages with no contents when doing form operations * Handle invalid page tree nodes when traversing pages

Fix bugs found by fuzz tests
* Several assertions in linearization were not always true; change them to run time errors * Handle a few cases of uninitialized objects * Handle pages with no contents when doing form operations * Handle invalid page tree nodes when traversing pages
Jay Berkenbilt
1 parent a35d4ce9
Showing 7 changed files with 67 additions and 35 deletions
TODO
libqpdf/QPDFFormFieldObjectHelper.cc
libqpdf/QPDFObjectHandle.cc
libqpdf/QPDFWriter.cc
libqpdf/QPDF_linearization.cc
libqpdf/QPDF_optimization.cc
qpdf/qtest/qpdf/no-contents-coalesce-contents.pdf
-OSS-Fuzz
-========
-
-* Fix open issues from https://oss-fuzz.com
-
 Next ABI
 ========
@@ -43,6 +43,10 @@ QPDFObjectHandle
 QPDFFormFieldObjectHelper::getInheritableFieldValue(std::string const& name)
 {
     QPDFObjectHandle node = this->oh;
+    if (! node.isDictionary())
+    {
+        return QPDFObjectHandle::newNull();
+    }
     QPDFObjectHandle result(node.getKey(name));
     std::set<QPDFObjGen> seen;
     while (result.isNull() && node.hasKey("/Parent"))
@@ -896,7 +900,8 @@ QPDFFormFieldObjectHelper::generateTextAppearance(
             QPDFObjectHandle dr = getInheritableFieldValue("/DR");
             font = getFontFromResource(dr, font_name);
         }
-        if (font.isDictionary() &&
+        if (font.isInitialized() &&
+            font.isDictionary() &&
             font.getKey("/Encoding").isName())
         {
             std::string encoding = font.getKey("/Encoding").getName();
@@ -1407,6 +1407,12 @@ QPDFObjectHandle::coalesceContentStreams()
         QTC::TC("qpdf", "QPDFObjectHandle coalesce called on stream");
         return;
     }
+    else if (! contents.isArray())
+    {
+        // /Contents is optional for pages, and some very damaged
+        // files may have pages that are invalid in other ways.
+        return;
+    }
     QPDF* qpdf = getOwningQPDF();
     if (qpdf == 0)
     {
@@ -851,9 +851,11 @@ QPDFWriter::parseVersion(std::string const&amp; version,
 	QUtil::int_to_string(minor);
     if (tmp != version)
     {
-	throw std::logic_error(
-	    "INTERNAL ERROR: QPDFWriter::parseVersion"
-	    " called with invalid version number " + version);
+        // The version number in the input is probably invalid. This
+        // happens with some files that are designed to exercise bugs,
+        // such as files in the fuzzer corpus. Unfortunately
+        // QPDFWriter doesn't have a way to give a warning, so we just
+        // ignore this case.
     }
 }
@@ -3013,7 +3015,7 @@ QPDFWriter::discardGeneration(std::map&lt;QPDFObjGen, int&gt; const&amp; in,
     {
         if (out.count((*iter).first.getObj()))
         {
-            throw std::logic_error(
+            throw std::runtime_error(
                 "QPDF cannot currently linearize files that contain"
                 " multiple objects with the same object ID and different"
                 " generations.  If you see this error message, please file"
@@ -3130,15 +3132,33 @@ QPDFWriter::writeLinearized()
     this->m->next_objid = part4_first_obj;
     enqueuePart(part4);
-    assert(this->m->next_objid == after_part4);
+    if (this->m->next_objid != after_part4)
+    {
+        // This can happen with very botched files as in the fuzzer
+        // test. There are likely some faulty assumptions in
+        // calculateLinearizationData
+        throw std::runtime_error(
+            "error encountered after"
+            " writing part 4 of linearized data");
+    }
     this->m->next_objid = part6_first_obj;
     enqueuePart(part6);
-    assert(this->m->next_objid == after_part6);
+    if (this->m->next_objid != after_part6)
+    {
+        throw std::runtime_error(
+            "error encountered after"
+            " writing part 6 of linearized data");
+    }
     this->m->next_objid = second_half_first_obj;
     enqueuePart(part7);
     enqueuePart(part8);
     enqueuePart(part9);
-    assert(this->m->next_objid == after_second_half);
+    if (this->m->next_objid != after_second_half)
+    {
+        throw std::runtime_error(
+            "error encountered after"
+            " writing part 9 of linearized data");
+    }
     qpdf_offset_t hint_length = 0;
     PointerHolder<Buffer> hint_buffer;
@@ -612,7 +612,7 @@ QPDF::checkLinearizationInternal()
     if (this->m->part6.empty())
     {
-        throw std::logic_error("linearization part 6 unexpectedly empty");
+        stopOnError("linearization part 6 unexpectedly empty");
     }
     qpdf_offset_t min_E = -1;
     qpdf_offset_t max_E = -1;
@@ -714,7 +714,7 @@ QPDF::getLinearizationOffset(QPDFObjGen const&amp; og)
 	break;
       default:
-	throw std::logic_error(
+	stopOnError(
 	    "getLinearizationOffset called for xref entry not of type 1 or 2");
 	break;
     }
@@ -862,7 +862,7 @@ QPDF::checkHPageOffset(std::list&lt;std::string&gt;&amp; errors,
 	    int idx = he.shared_identifiers.at(i);
 	    if (shared_idx_to_obj.count(idx) == 0)
             {
-                throw std::logic_error(
+                stopOnError(
                     "unable to get object for item in"
                     " shared objects hint table");
             }
@@ -874,7 +874,7 @@ QPDF::checkHPageOffset(std::list&lt;std::string&gt;&amp; errors,
 	    int idx = ce.shared_identifiers.at(i);
 	    if (idx >= this->m->c_shared_object_data.nshared_total)
             {
-                throw std::logic_error(
+                stopOnError(
                     "index out of bounds for shared object hint table");
             }
 	    int obj = this->m->c_shared_object_data.entries.at(toS(idx)).object;
@@ -1444,7 +1444,7 @@ QPDF::calculateLinearizationData(std::map&lt;int, int&gt; const&amp; object_stream_data)
 		break;
 	      case ObjUser::ou_bad:
-		throw std::logic_error(
+		stopOnError(
 		    "INTERNAL ERROR: QPDF::calculateLinearizationData: "
 		    "invalid user type");
 		break;
@@ -1558,10 +1558,14 @@ QPDF::calculateLinearizationData(std::map&lt;int, int&gt; const&amp; object_stream_data)
     // will do the same.
     // First, place the actual first page object itself.
+    if (pages.empty())
+    {
+        stopOnError("no pages found while calculating linearization data");
+    }
     QPDFObjGen first_page_og(pages.at(0).getObjGen());
     if (! lc_first_page_private.count(first_page_og))
     {
-	throw std::logic_error(
+	stopOnError(
 	    "INTERNAL ERROR: QPDF::calculateLinearizationData: first page "
 	    "object not in lc_first_page_private");
     }
@@ -1611,7 +1615,7 @@ QPDF::calculateLinearizationData(std::map&lt;int, int&gt; const&amp; object_stream_data)
 	QPDFObjGen page_og(pages.at(i).getObjGen());
 	if (! lc_other_page_private.count(page_og))
 	{
-	    throw std::logic_error(
+	    stopOnError(
 		"INTERNAL ERROR: "
 		"QPDF::calculateLinearizationData: page object for page " +
 		QUtil::uint_to_string(i) + " not in lc_other_page_private");
@@ -1646,7 +1650,7 @@ QPDF::calculateLinearizationData(std::map&lt;int, int&gt; const&amp; object_stream_data)
     // That should have covered all part7 objects.
     if (! lc_other_page_private.empty())
     {
-	throw std::logic_error(
+	stopOnError(
 	    "INTERNAL ERROR:"
 	    " QPDF::calculateLinearizationData: lc_other_page_private is "
 	    "not empty after generation of part7");
@@ -1731,7 +1735,7 @@ QPDF::calculateLinearizationData(std::map&lt;int, int&gt; const&amp; object_stream_data)
     }
     if (! lc_thumbnail_private.empty())
     {
-	throw std::logic_error(
+	stopOnError(
 	    "INTERNAL ERROR: "
 	    "QPDF::calculateLinearizationData: lc_thumbnail_private "
 	    "not empty after placing thumbnails");
@@ -1765,7 +1769,7 @@ QPDF::calculateLinearizationData(std::map&lt;int, int&gt; const&amp; object_stream_data)
     size_t num_wanted = this->m->object_to_obj_users.size();
     if (num_placed != num_wanted)
     {
-	throw std::logic_error(
+	stopOnError(
 	    "INTERNAL ERROR: QPDF::calculateLinearizationData: wrong "
 	    "number of objects placed (num_placed = " +
 	    QUtil::uint_to_string(num_placed) +
@@ -1820,7 +1824,7 @@ QPDF::calculateLinearizationData(std::map&lt;int, int&gt; const&amp; object_stream_data)
     if (static_cast<size_t>(this->m->c_shared_object_data.nshared_total) !=
         this->m->c_shared_object_data.entries.size())
     {
-        throw std::logic_error(
+        stopOnError(
             "shared object hint table has wrong number of entries");
     }
@@ -2058,7 +2062,7 @@ QPDF::calculateHSharedObject(
     }
     if (soe.size() != static_cast<size_t>(cso.nshared_total))
     {
-        throw std::logic_error("soe has wrong size after initialization");
+        stopOnError("soe has wrong size after initialization");
     }
     so.nshared_total = cso.nshared_total;
@@ -195,6 +195,14 @@ QPDF::pushInheritedAttributesToPageInternal(
     }
     visited.insert(this_og);
+    if (! cur_pages.isDictionary())
+    {
+	throw QPDFExc(qpdf_e_damaged_pdf, this->m->file->getName(),
+		      this->m->last_object_description,
+		      this->m->file->getLastOffset(),
+		      "invalid object in page tree");
+    }
+
     // Extract the underlying dictionary object
     std::string type = cur_pages.getKey("/Type").getName();
@@ -7,21 +7,15 @@ endobj
 << /Count 1 /Kids [ 3 0 R ] /Type /Pages >>
 endobj
 3 0 obj
-<< /Contents 4 0 R /MediaBox [ 0 0 720 720 ] /Parent 2 0 R /Resources << >> /Type /Page >>
-endobj
-4 0 obj
-<< /Length 0 /Filter /FlateDecode >>
-stream
-endstream
+<< /MediaBox [ 0 0 720 720 ] /Parent 2 0 R /Resources << >> /Type /Page >>
 endobj
 xref
-0 5
+0 4
 0000000000 65535 f 
 0000000015 00000 n 
 0000000064 00000 n 
 0000000123 00000 n 
-0000000229 00000 n 
-trailer << /Root 1 0 R /Size 5 /ID [<52bba3c78160d0c6e851b59110e5d076><31415926535897932384626433832795>] >>
+trailer << /Root 1 0 R /Size 4 /ID [<52bba3c78160d0c6e851b59110e5d076><31415926535897932384626433832795>] >>
 startxref
-298
+213
 %%EOF