diff options
| -rw-r--r-- | ChangeLog | 5 | ||||
| -rw-r--r-- | libqpdf/Pl_LZWDecoder.cc | 31 | ||||
| -rw-r--r-- | libqpdf/QPDFWriter.cc | 29 | ||||
| -rw-r--r-- | libqpdf/QPDF_linearization.cc | 33 | ||||
| -rw-r--r-- | libqpdf/QPDF_pages.cc | 5 |
5 files changed, 82 insertions, 21 deletions
@@ -1,5 +1,10 @@ 2013-10-05 Jay Berkenbilt <ejb@ql.org> + * Replace some assert() calls with std::logic_error exceptions. + Ideally there shouldn't be assert() calls outside of testing. + This change may make a few more potential code errors in handling + invalid data recoverable. + * Security fix: In places where std::vector<T>(size_t) was used, either validate that the size parameter is sane or refactor code to avoid the need to pre-allocate the vector. This reduces the diff --git a/libqpdf/Pl_LZWDecoder.cc b/libqpdf/Pl_LZWDecoder.cc index f7c6fb30..82a668e3 100644 --- a/libqpdf/Pl_LZWDecoder.cc +++ b/libqpdf/Pl_LZWDecoder.cc @@ -1,6 +1,7 @@ #include <qpdf/Pl_LZWDecoder.hh> #include <qpdf/QTC.hh> +#include <qpdf/QUtil.hh> #include <stdexcept> #include <string.h> #include <assert.h> @@ -100,14 +101,23 @@ Pl_LZWDecoder::getFirstChar(int code) { result = static_cast<unsigned char>(code); } - else + else if (code > 257) { - assert(code > 257); unsigned int idx = code - 258; - assert(idx < table.size()); + if (idx >= table.size()) + { + throw std::logic_error( + "Pl_LZWDecoder::getFirstChar: table overflow"); + } Buffer& b = table[idx]; result = b.getBuffer()[0]; } + else + { + throw std::logic_error( + "Pl_LZWDecoder::getFirstChar called with invalid code (" + + QUtil::int_to_string(code) + ")"); + } return result; } @@ -124,15 +134,24 @@ Pl_LZWDecoder::addToTable(unsigned char next) last_data = tmp; last_size = 1; } - else + else if (this->last_code > 257) { - assert(this->last_code > 257); unsigned int idx = this->last_code - 258; - assert(idx < table.size()); + if (idx >= table.size()) + { + throw std::logic_error( + "Pl_LZWDecoder::addToTable: table overflow"); + } Buffer& b = table[idx]; last_data = b.getBuffer(); last_size = b.getSize(); } + else + { + throw std::logic_error( + "Pl_LZWDecoder::addToTable called with invalid code (" + + QUtil::int_to_string(this->last_code) + ")"); + } Buffer entry(1 + last_size); unsigned char* new_data = entry.getBuffer(); diff --git a/libqpdf/QPDFWriter.cc b/libqpdf/QPDFWriter.cc index 863e753b..949652c5 100644 --- a/libqpdf/QPDFWriter.cc +++ b/libqpdf/QPDFWriter.cc @@ -772,7 +772,11 @@ QPDFWriter::bytesNeeded(unsigned long long n) void QPDFWriter::writeBinary(unsigned long long val, unsigned int bytes) { - assert(bytes <= sizeof(unsigned long long)); + if (bytes > sizeof(unsigned long long)) + { + throw std::logic_error( + "QPDFWriter::writeBinary called with too many bytes"); + } unsigned char data[sizeof(unsigned long long)]; for (unsigned int i = 0; i < bytes; ++i) { @@ -1097,7 +1101,11 @@ QPDFWriter::writeTrailer(trailer_e which, int size, bool xref_stream, qpdf_offset_t pos = this->pipeline->getCount(); writeString(QUtil::int_to_string(prev)); int nspaces = pos - this->pipeline->getCount() + 21; - assert(nspaces >= 0); + if (nspaces < 0) + { + throw std::logic_error( + "QPDFWriter: no padding required in trailer"); + } writePad(nspaces); } } @@ -2814,9 +2822,11 @@ QPDFWriter::writeLinearized() // place as in pass 1. writePad(first_xref_end - endpos); - // A failure of this insertion means we didn't allow - // enough padding for the first pass xref stream. - assert(this->pipeline->getCount() == first_xref_end); + if (this->pipeline->getCount() != first_xref_end) + { + throw std::logic_error( + "insufficient padding for first pass xref stream"); + } } writeString("\n"); } @@ -2897,8 +2907,13 @@ QPDFWriter::writeLinearized() // If this assertion fails, maybe we didn't have // enough padding above. - assert(this->pipeline->getCount() == - second_xref_end + hint_length); + if (this->pipeline->getCount() != + second_xref_end + hint_length) + { + throw std::logic_error( + "count mismatch after xref stream;" + " possible insufficient padding?"); + } } } else diff --git a/libqpdf/QPDF_linearization.cc b/libqpdf/QPDF_linearization.cc index 7d2560d3..4b28e480 100644 --- a/libqpdf/QPDF_linearization.cc +++ b/libqpdf/QPDF_linearization.cc @@ -606,15 +606,21 @@ QPDF::checkLinearizationInternal() // agree with pdlin. As of this writing, the test suite doesn't // contain any files with threads. - assert(! this->part6.empty()); + if (this->part6.empty()) + { + throw std::logic_error("linearization part 6 unexpectedly empty"); + } qpdf_offset_t min_E = -1; qpdf_offset_t max_E = -1; for (std::vector<QPDFObjectHandle>::iterator iter = this->part6.begin(); iter != this->part6.end(); ++iter) { QPDFObjGen og((*iter).getObjGen()); - // All objects have to have been dereferenced to be classified. - assert(this->obj_cache.count(og) > 0); + if (this->obj_cache.count(og) == 0) + { + // All objects have to have been dereferenced to be classified. + throw std::logic_error("linearization part6 object not in cache"); + } ObjCache const& oc = this->obj_cache[og]; min_E = std::max(min_E, oc.end_before_space); max_E = std::max(max_E, oc.end_after_space); @@ -832,14 +838,23 @@ QPDF::checkHPageOffset(std::list<std::string>& errors, for (int i = 0; i < he.nshared_objects; ++i) { int idx = he.shared_identifiers[i]; - assert(shared_idx_to_obj.count(idx) > 0); + if (shared_idx_to_obj.count(idx) == 0) + { + throw std::logic_error( + "unable to get object for item in" + " shared objects hint table"); + } hint_shared.insert(shared_idx_to_obj[idx]); } for (int i = 0; i < ce.nshared_objects; ++i) { int idx = ce.shared_identifiers[i]; - assert(idx < this->c_shared_object_data.nshared_total); + if (idx >= this->c_shared_object_data.nshared_total) + { + throw std::logic_error( + "index out of bounds for shared object hint table"); + } int obj = this->c_shared_object_data.entries[idx].object; computed_shared.insert(obj); } @@ -1754,8 +1769,12 @@ QPDF::calculateLinearizationData(std::map<int, int> const& object_stream_data) shared.push_back(CHSharedObjectEntry(obj)); } } - assert(static_cast<size_t>(this->c_shared_object_data.nshared_total) == - this->c_shared_object_data.entries.size()); + if (static_cast<size_t>(this->c_shared_object_data.nshared_total) != + this->c_shared_object_data.entries.size()) + { + throw std::logic_error( + "shared object hint table has wrong number of entries"); + } // Now compute the list of shared objects for each page after the // first page. diff --git a/libqpdf/QPDF_pages.cc b/libqpdf/QPDF_pages.cc index 0604cd40..b88ed5fa 100644 --- a/libqpdf/QPDF_pages.cc +++ b/libqpdf/QPDF_pages.cc @@ -120,7 +120,10 @@ QPDF::flattenPagesTree() pages.replaceKey("/Kids", QPDFObjectHandle::newArray(this->all_pages)); // /Count has not changed - assert(pages.getKey("/Count").getIntValue() == len); + if (pages.getKey("/Count").getIntValue() != len) + { + throw std::logic_error("/Count is wrong after flattening pages tree"); + } } void |