Commit fe1fffe8dba0926b7a738ed2195590b7db18dcab
1 parent
bcf81a14
Change QPDF max_warnings into a hard limit
Throw damagedFile if max_warnings is exceeded. Change qpdf_fuzzer warnings limit to limit to 500.
Showing
4 changed files
with
15 additions
and
22 deletions
fuzz/qpdf_fuzzer.cc
| @@ -57,7 +57,7 @@ FuzzHelper::getQpdf() | @@ -57,7 +57,7 @@ FuzzHelper::getQpdf() | ||
| 57 | auto is = | 57 | auto is = |
| 58 | std::shared_ptr<InputSource>(new BufferInputSource("fuzz input", &this->input_buffer)); | 58 | std::shared_ptr<InputSource>(new BufferInputSource("fuzz input", &this->input_buffer)); |
| 59 | auto qpdf = QPDF::create(); | 59 | auto qpdf = QPDF::create(); |
| 60 | - qpdf->setMaxWarnings(20); | 60 | + qpdf->setMaxWarnings(500); |
| 61 | qpdf->processInputSource(is); | 61 | qpdf->processInputSource(is); |
| 62 | return qpdf; | 62 | return qpdf; |
| 63 | } | 63 | } |
include/qpdf/QPDF.hh
| @@ -228,9 +228,9 @@ class QPDF | @@ -228,9 +228,9 @@ class QPDF | ||
| 228 | QPDF_DLL | 228 | QPDF_DLL |
| 229 | void setSuppressWarnings(bool); | 229 | void setSuppressWarnings(bool); |
| 230 | 230 | ||
| 231 | - // Set the maximum number of warnings to output. Subsequent warnings are suppressed. | 231 | + // Set the maximum number of warnings. A QPDFExc is thrown if the limit is exceeded. |
| 232 | QPDF_DLL | 232 | QPDF_DLL |
| 233 | - void setMaxWarnings(int); | 233 | + void setMaxWarnings(size_t); |
| 234 | 234 | ||
| 235 | // By default, QPDF will try to recover if it finds certain types of errors in PDF files. If | 235 | // By default, QPDF will try to recover if it finds certain types of errors in PDF files. If |
| 236 | // turned off, it will throw an exception on the first such problem it finds without attempting | 236 | // turned off, it will throw an exception on the first such problem it finds without attempting |
| @@ -1501,7 +1501,7 @@ class QPDF | @@ -1501,7 +1501,7 @@ class QPDF | ||
| 1501 | bool provided_password_is_hex_key{false}; | 1501 | bool provided_password_is_hex_key{false}; |
| 1502 | bool ignore_xref_streams{false}; | 1502 | bool ignore_xref_streams{false}; |
| 1503 | bool suppress_warnings{false}; | 1503 | bool suppress_warnings{false}; |
| 1504 | - int max_warnings{0}; | 1504 | + size_t max_warnings{0}; |
| 1505 | bool attempt_recovery{true}; | 1505 | bool attempt_recovery{true}; |
| 1506 | bool check_mode{false}; | 1506 | bool check_mode{false}; |
| 1507 | std::shared_ptr<EncryptionParameters> encp; | 1507 | std::shared_ptr<EncryptionParameters> encp; |
libqpdf/QPDF.cc
| @@ -332,7 +332,7 @@ QPDF::setSuppressWarnings(bool val) | @@ -332,7 +332,7 @@ QPDF::setSuppressWarnings(bool val) | ||
| 332 | } | 332 | } |
| 333 | 333 | ||
| 334 | void | 334 | void |
| 335 | -QPDF::setMaxWarnings(int val) | 335 | +QPDF::setMaxWarnings(size_t val) |
| 336 | { | 336 | { |
| 337 | m->max_warnings = val; | 337 | m->max_warnings = val; |
| 338 | } | 338 | } |
| @@ -504,13 +504,11 @@ QPDF::inParse(bool v) | @@ -504,13 +504,11 @@ QPDF::inParse(bool v) | ||
| 504 | void | 504 | void |
| 505 | QPDF::warn(QPDFExc const& e) | 505 | QPDF::warn(QPDFExc const& e) |
| 506 | { | 506 | { |
| 507 | + if (m->max_warnings > 0 && m->warnings.size() >= m->max_warnings) { | ||
| 508 | + stopOnError("Too many warnings - file is too badly damaged"); | ||
| 509 | + } | ||
| 507 | m->warnings.push_back(e); | 510 | m->warnings.push_back(e); |
| 508 | if (!m->suppress_warnings) { | 511 | if (!m->suppress_warnings) { |
| 509 | - if (m->max_warnings > 0 && m->warnings.size() > 20) { | ||
| 510 | - *m->log->getWarn() << "WARNING: too many warnings - additional warnings suppressed\n"; | ||
| 511 | - m->suppress_warnings = true; | ||
| 512 | - return; | ||
| 513 | - } | ||
| 514 | *m->log->getWarn() << "WARNING: " << m->warnings.back().what() << "\n"; | 512 | *m->log->getWarn() << "WARNING: " << m->warnings.back().what() << "\n"; |
| 515 | } | 513 | } |
| 516 | } | 514 | } |
libqpdf/QPDF_json.cc
| @@ -233,13 +233,12 @@ provide_data(std::shared_ptr<InputSource> is, qpdf_offset_t start, qpdf_offset_t | @@ -233,13 +233,12 @@ provide_data(std::shared_ptr<InputSource> is, qpdf_offset_t start, qpdf_offset_t | ||
| 233 | class QPDF::JSONReactor: public JSON::Reactor | 233 | class QPDF::JSONReactor: public JSON::Reactor |
| 234 | { | 234 | { |
| 235 | public: | 235 | public: |
| 236 | - JSONReactor(QPDF& pdf, std::shared_ptr<InputSource> is, bool must_be_complete, int max_warnings) : | 236 | + JSONReactor(QPDF& pdf, std::shared_ptr<InputSource> is, bool must_be_complete) : |
| 237 | pdf(pdf), | 237 | pdf(pdf), |
| 238 | is(is), | 238 | is(is), |
| 239 | must_be_complete(must_be_complete), | 239 | must_be_complete(must_be_complete), |
| 240 | descr(std::make_shared<QPDFValue::Description>( | 240 | descr(std::make_shared<QPDFValue::Description>( |
| 241 | - QPDFValue::JSON_Descr(std::make_shared<std::string>(is->getName()), ""))), | ||
| 242 | - max_warnings(max_warnings) | 241 | + QPDFValue::JSON_Descr(std::make_shared<std::string>(is->getName()), ""))) |
| 243 | { | 242 | { |
| 244 | for (auto& oc: pdf.m->obj_cache) { | 243 | for (auto& oc: pdf.m->obj_cache) { |
| 245 | if (oc.second.object->getTypeCode() == ::ot_reserved) { | 244 | if (oc.second.object->getTypeCode() == ::ot_reserved) { |
| @@ -292,8 +291,7 @@ class QPDF::JSONReactor: public JSON::Reactor | @@ -292,8 +291,7 @@ class QPDF::JSONReactor: public JSON::Reactor | ||
| 292 | std::shared_ptr<InputSource> is; | 291 | std::shared_ptr<InputSource> is; |
| 293 | bool must_be_complete{true}; | 292 | bool must_be_complete{true}; |
| 294 | std::shared_ptr<QPDFValue::Description> descr; | 293 | std::shared_ptr<QPDFValue::Description> descr; |
| 295 | - int errors{0}; | ||
| 296 | - int max_warnings{0}; | 294 | + bool errors{false}; |
| 297 | bool saw_qpdf{false}; | 295 | bool saw_qpdf{false}; |
| 298 | bool saw_qpdf_meta{false}; | 296 | bool saw_qpdf_meta{false}; |
| 299 | bool saw_objects{false}; | 297 | bool saw_objects{false}; |
| @@ -316,21 +314,18 @@ class QPDF::JSONReactor: public JSON::Reactor | @@ -316,21 +314,18 @@ class QPDF::JSONReactor: public JSON::Reactor | ||
| 316 | void | 314 | void |
| 317 | QPDF::JSONReactor::error(qpdf_offset_t offset, std::string const& msg) | 315 | QPDF::JSONReactor::error(qpdf_offset_t offset, std::string const& msg) |
| 318 | { | 316 | { |
| 319 | - ++errors; | 317 | + errors = true; |
| 320 | std::string object = this->cur_object; | 318 | std::string object = this->cur_object; |
| 321 | if (is->getName() != pdf.getFilename()) { | 319 | if (is->getName() != pdf.getFilename()) { |
| 322 | object += " from " + is->getName(); | 320 | object += " from " + is->getName(); |
| 323 | } | 321 | } |
| 324 | - this->pdf.warn(qpdf_e_json, object, offset, msg); | ||
| 325 | - if (max_warnings > 0 && errors >= max_warnings) { | ||
| 326 | - throw std::runtime_error("errors found in JSON"); | ||
| 327 | - } | 322 | + pdf.warn(qpdf_e_json, object, offset, msg); |
| 328 | } | 323 | } |
| 329 | 324 | ||
| 330 | bool | 325 | bool |
| 331 | QPDF::JSONReactor::anyErrors() const | 326 | QPDF::JSONReactor::anyErrors() const |
| 332 | { | 327 | { |
| 333 | - return errors > 0; | 328 | + return errors; |
| 334 | } | 329 | } |
| 335 | 330 | ||
| 336 | void | 331 | void |
| @@ -825,7 +820,7 @@ QPDF::updateFromJSON(std::shared_ptr<InputSource> is) | @@ -825,7 +820,7 @@ QPDF::updateFromJSON(std::shared_ptr<InputSource> is) | ||
| 825 | void | 820 | void |
| 826 | QPDF::importJSON(std::shared_ptr<InputSource> is, bool must_be_complete) | 821 | QPDF::importJSON(std::shared_ptr<InputSource> is, bool must_be_complete) |
| 827 | { | 822 | { |
| 828 | - JSONReactor reactor(*this, is, must_be_complete, m->max_warnings); | 823 | + JSONReactor reactor(*this, is, must_be_complete); |
| 829 | try { | 824 | try { |
| 830 | JSON::parse(*is, &reactor); | 825 | JSON::parse(*is, &reactor); |
| 831 | } catch (std::runtime_error& e) { | 826 | } catch (std::runtime_error& e) { |