From b07ad6794eea175ee7f4b8c505995bdb4f397ce6 Mon Sep 17 00:00:00 2001 From: Jay Berkenbilt Date: Fri, 21 Jun 2019 17:37:33 -0400 Subject: 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 --- libqpdf/QPDFFormFieldObjectHelper.cc | 7 ++++++- libqpdf/QPDFObjectHandle.cc | 6 ++++++ libqpdf/QPDFWriter.cc | 34 +++++++++++++++++++++++++++------- libqpdf/QPDF_linearization.cc | 28 ++++++++++++++++------------ libqpdf/QPDF_optimization.cc | 8 ++++++++ 5 files changed, 63 insertions(+), 20 deletions(-) (limited to 'libqpdf') diff --git a/libqpdf/QPDFFormFieldObjectHelper.cc b/libqpdf/QPDFFormFieldObjectHelper.cc index b3c61ce5..97257c85 100644 --- a/libqpdf/QPDFFormFieldObjectHelper.cc +++ b/libqpdf/QPDFFormFieldObjectHelper.cc @@ -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 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(); diff --git a/libqpdf/QPDFObjectHandle.cc b/libqpdf/QPDFObjectHandle.cc index 9ccfa37a..a3147940 100644 --- a/libqpdf/QPDFObjectHandle.cc +++ b/libqpdf/QPDFObjectHandle.cc @@ -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) { diff --git a/libqpdf/QPDFWriter.cc b/libqpdf/QPDFWriter.cc index 23a67d7c..93a634f2 100644 --- a/libqpdf/QPDFWriter.cc +++ b/libqpdf/QPDFWriter.cc @@ -851,9 +851,11 @@ QPDFWriter::parseVersion(std::string const& 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 const& 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 hint_buffer; diff --git a/libqpdf/QPDF_linearization.cc b/libqpdf/QPDF_linearization.cc index 04b646d3..5eb6277e 100644 --- a/libqpdf/QPDF_linearization.cc +++ b/libqpdf/QPDF_linearization.cc @@ -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& 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& 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& 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 const& 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 const& 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 const& 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 const& 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 const& 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 const& 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 const& object_stream_data) if (static_cast(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(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; diff --git a/libqpdf/QPDF_optimization.cc b/libqpdf/QPDF_optimization.cc index 3394836c..afa7ccbd 100644 --- a/libqpdf/QPDF_optimization.cc +++ b/libqpdf/QPDF_optimization.cc @@ -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(); -- cgit v1.2.3-54-g00ecf