From f1800410b6c1f1510890b97fe5e8ce962b69ac94 Mon Sep 17 00:00:00 2001 From: m-holger Date: Thu, 16 Jan 2025 16:36:48 +0000 Subject: [PATCH] Revert "Merge pull request #1289 from m-holger/fuzz" --- fuzz/CMakeLists.txt | 3 --- fuzz/qpdf_extra/99999a.fuzz | 63 --------------------------------------------------------------- fuzz/qpdf_extra/99999b.fuzz | Bin 81 -> 0 bytes fuzz/qpdf_extra/99999c.fuzz | Bin 13650 -> 0 bytes fuzz/qtest/fuzz.test | 2 +- libqpdf/QPDF.cc | 59 +++++++++++++++++++++++++++-------------------------------- libqpdf/qpdf/QPDF_private.hh | 4 ++-- qpdf/qpdf.testcov | 1 - qpdf/qtest/qpdf/issue-fuzz.out | 19 ------------------- qpdf/qtest/qpdf/issue-fuzz.pdf | Bin 81 -> 0 bytes qpdf/qtest/specific-bugs.test | 1 - 11 files changed, 30 insertions(+), 122 deletions(-) delete mode 100644 fuzz/qpdf_extra/99999a.fuzz delete mode 100644 fuzz/qpdf_extra/99999b.fuzz delete mode 100644 fuzz/qpdf_extra/99999c.fuzz delete mode 100644 qpdf/qtest/qpdf/issue-fuzz.out delete mode 100644 qpdf/qtest/qpdf/issue-fuzz.pdf diff --git a/fuzz/CMakeLists.txt b/fuzz/CMakeLists.txt index 6b3cccf..029ed05 100644 --- a/fuzz/CMakeLists.txt +++ b/fuzz/CMakeLists.txt @@ -142,9 +142,6 @@ set(CORPUS_OTHER 70306b.fuzz 71624.fuzz 71689.fuzz - 99999a.fuzz - 99999b.fuzz - 99999c.fuzz 99999d.fuzz 99999e.fuzz 369662293.fuzz diff --git a/fuzz/qpdf_extra/99999a.fuzz b/fuzz/qpdf_extra/99999a.fuzz deleted file mode 100644 index 026c742..0000000 --- a/fuzz/qpdf_extra/99999a.fuzz +++ /dev/null @@ -1,63 +0,0 @@ -%PDF-1.5 -%€€€€ -1 0 obj -<< - /Type /Catalog - /Pages 2 0 R ->> -endobj -2 0 obj -<< - /Count 6 Ri - 0K/ds [3 0 R] - /Type /Pages ->> -endobj -3 0 obj -<< - /Resources << - /Font << - /F1 5 0 R - >> - >> - /MediaBox [0 0 795 842] - /Parent 2 0 R - /Contents 4 0 R - /Type /Page -=> -endobj -4 0 obj -<<444444444444444444444444 1 Tr /F1 30 Tf 350 750 Td (foobar) Tj ET -endstream -endobj -5 0 obj -<< - /Name /F1 - /BaseFont /Helvetica - /Type /Font - /Subtype /Type1 ->> -e„dobj -6 0 obj -<< /Length 6 0 R >> -stre444444444444444444444444444444<<>> -endobj -xref -0 8 -0000000000 65535 f -0000000015 00000 n -0000000066 00000 n -0000000130 00000 n -0000000269 00000 n -0000000362 00000 n -000000ÎËËÉßÏÏÏ00 n -0000000500 00000 n -trailer -<< - /Size 713115528178535 - /Root 1 0 R - /Info 7 0 R ->> -startxref -520 -%%EOF \ No newline at end of file diff --git a/fuzz/qpdf_extra/99999b.fuzz b/fuzz/qpdf_extra/99999b.fuzz deleted file mode 100644 index 288a6b5..0000000 Binary files a/fuzz/qpdf_extra/99999b.fuzz and /dev/null differ diff --git a/fuzz/qpdf_extra/99999c.fuzz b/fuzz/qpdf_extra/99999c.fuzz deleted file mode 100644 index c856648..0000000 Binary files a/fuzz/qpdf_extra/99999c.fuzz and /dev/null differ diff --git a/fuzz/qtest/fuzz.test b/fuzz/qtest/fuzz.test index efffdc6..cf38a63 100644 --- a/fuzz/qtest/fuzz.test +++ b/fuzz/qtest/fuzz.test @@ -11,7 +11,7 @@ my $td = new TestDriver('fuzz'); my $qpdf_corpus = $ENV{'QPDF_FUZZ_CORPUS'} || die "must set QPDF_FUZZ_CORPUS"; -my $n_qpdf_files = 87; # increment when adding new files +my $n_qpdf_files = 84; # increment when adding new files my @fuzzers = ( ['ascii85' => 1], diff --git a/libqpdf/QPDF.cc b/libqpdf/QPDF.cc index 12b23a8..a5f3185 100644 --- a/libqpdf/QPDF.cc +++ b/libqpdf/QPDF.cc @@ -832,6 +832,10 @@ std::vector QPDF::Xref_table::bad_subsections(std::string& line, qpdf_offset_t start) { std::vector result; + qpdf_offset_t f1 = 0; + int f2 = 0; + char type = '\0'; + file->seek(start, SEEK_SET); while (true) { @@ -840,7 +844,7 @@ QPDF::Xref_table::bad_subsections(std::string& line, qpdf_offset_t start) auto [obj, num, offset] = result.emplace_back(subsection(line)); file->seek(offset, SEEK_SET); for (qpdf_offset_t i = obj; i - num < obj; ++i) { - if (!std::get<0>(read_entry())) { + if (!read_entry(f1, f2, type)) { QTC::TC("qpdf", "QPDF invalid xref entry"); throw damaged_table("invalid xref entry (obj=" + std::to_string(i) + ")"); } @@ -886,13 +890,9 @@ QPDF::Xref_table::subsections(std::string& line) } } -// Returns (success, f1, f2, type). -std::tuple -QPDF::Xref_table::read_bad_entry() +bool +QPDF::Xref_table::read_bad_entry(qpdf_offset_t& f1, int& f2, char& type) { - qpdf_offset_t f1{0}; - int f2{0}; - char type{'\0'}; // Reposition after initial read attempt and reread. file->seek(file->getLastOffset(), SEEK_SET); auto line = file->readLine(30); @@ -910,7 +910,7 @@ QPDF::Xref_table::read_bad_entry() } // Require digit if (!QUtil::is_digit(*p)) { - return {false, 0, 0, '\0'}; + return false; } // Gather digits std::string f1_str; @@ -919,7 +919,7 @@ QPDF::Xref_table::read_bad_entry() } // Require space if (!QUtil::is_space(*p)) { - return {false, 0, 0, '\0'}; + return false; } if (QUtil::is_space(*(p + 1))) { QTC::TC("qpdf", "QPDF ignore first extra space in xref entry"); @@ -931,7 +931,7 @@ QPDF::Xref_table::read_bad_entry() } // Require digit if (!QUtil::is_digit(*p)) { - return {false, 0, 0, '\0'}; + return false; } // Gather digits std::string f2_str; @@ -940,7 +940,7 @@ QPDF::Xref_table::read_bad_entry() } // Require space if (!QUtil::is_space(*p)) { - return {false, 0, 0, '\0'}; + return false; } if (QUtil::is_space(*(p + 1))) { QTC::TC("qpdf", "QPDF ignore second extra space in xref entry"); @@ -953,7 +953,7 @@ QPDF::Xref_table::read_bad_entry() if ((*p == 'f') || (*p == 'n')) { type = *p; } else { - return {false, 0, 0, '\0'}; + return false; } if ((f1_str.length() != 10) || (f2_str.length() != 5)) { QTC::TC("qpdf", "QPDF ignore length error xref entry"); @@ -967,23 +967,18 @@ QPDF::Xref_table::read_bad_entry() f1 = QUtil::string_to_ll(f1_str.c_str()); f2 = QUtil::string_to_int(f2_str.c_str()); - return {true, f1, f2, type}; + return true; } // Optimistically read and parse xref entry. If entry is bad, call read_bad_xrefEntry and return -// result. Returns (success, f1, f2, type). -std::tuple -QPDF::Xref_table::read_entry() +// result. +bool +QPDF::Xref_table::read_entry(qpdf_offset_t& f1, int& f2, char& type) { - qpdf_offset_t f1{0}; - int f2{0}; - char type{'\0'}; std::array line; - f1 = 0; - f2 = 0; if (file->read(line.data(), 20) != 20) { // C++20: [[unlikely]] - return {false, 0, 0, '\0'}; + return false; } line[20] = '\0'; char const* p = line.data(); @@ -1007,7 +1002,7 @@ QPDF::Xref_table::read_entry() if (!QUtil::is_space(*p++)) { // Entry doesn't start with space or digit. // C++20: [[unlikely]] - return {false, 0, 0, '\0'}; + return false; } // Gather digits. NB No risk of overflow as 99'999 < max int. while (*p == '0') { @@ -1024,10 +1019,10 @@ QPDF::Xref_table::read_entry() // No test for valid line[19]. if (*(++p) && *(++p) && (*p == '\n' || *p == '\r') && f1_len == 10 && f2_len == 5) { // C++20: [[likely]] - return {true, f1, f2, type}; + return true; } } - return read_bad_entry(); + return read_bad_entry(f1, f2, type); } // Read a single cross-reference table section and associated trailer. @@ -1057,10 +1052,7 @@ QPDF::Xref_table::process_section(qpdf_offset_t xref_offset) QTC::TC("qpdf", "QPDF trailer size not integer"); throw qpdf.damagedPDF("trailer", "/Size key in trailer dictionary is not an integer"); } - if (sz >= static_cast(max_id_)) { - QTC::TC("qpdf", "QPDF trailer size impossibly large"); - throw qpdf.damagedPDF("trailer", "/Size key in trailer dictionary is impossibly large"); - } + table.resize(sz); } @@ -1072,8 +1064,10 @@ QPDF::Xref_table::process_section(qpdf_offset_t xref_offset) first_item_offset_ = file->tell(); } // For xref_table, these will always be small enough to be ints - auto [success, f1, f2, type] = read_entry(); - if (!success) { + qpdf_offset_t f1 = 0; + int f2 = 0; + char type = '\0'; + if (!read_entry(f1, f2, type)) { throw damaged_table("invalid xref entry (obj=" + std::to_string(i) + ")"); } if (type == 'f') { @@ -1594,7 +1588,8 @@ QPDF::Xref_table::read_trailer() { qpdf_offset_t offset = file->tell(); bool empty = false; - auto object = QPDFParser(*file, "trailer", tokenizer, nullptr, &qpdf, true).parse(empty, false); + auto object = + QPDFParser(*file, "trailer", tokenizer, nullptr, &qpdf, true).parse(empty, false); if (empty) { // Nothing in the PDF spec appears to allow empty objects, but they have been encountered in // actual PDF files and Adobe Reader appears to ignore them. diff --git a/libqpdf/qpdf/QPDF_private.hh b/libqpdf/qpdf/QPDF_private.hh index fa14fdc..b055763 100644 --- a/libqpdf/qpdf/QPDF_private.hh +++ b/libqpdf/qpdf/QPDF_private.hh @@ -292,8 +292,8 @@ class QPDF::Xref_table std::vector subsections(std::string& line); std::vector bad_subsections(std::string& line, qpdf_offset_t offset); Subsection subsection(std::string const& line); - std::tuple read_entry(); - std::tuple read_bad_entry(); + bool read_entry(qpdf_offset_t& f1, int& f2, char& type); + bool read_bad_entry(qpdf_offset_t& f1, int& f2, char& type); // Methods to parse streams qpdf_offset_t read_stream(qpdf_offset_t offset); diff --git a/qpdf/qpdf.testcov b/qpdf/qpdf.testcov index 6f8d556..09593c9 100644 --- a/qpdf/qpdf.testcov +++ b/qpdf/qpdf.testcov @@ -55,7 +55,6 @@ QPDF invalid xref entry 0 QPDF missing trailer 0 QPDF trailer lacks size 0 QPDF trailer size not integer 0 -QPDF trailer size impossibly large 0 QPDF trailer prev not integer 0 QPDFParser bad brace 0 QPDFParser bad brace in parseRemainder 0 diff --git a/qpdf/qtest/qpdf/issue-fuzz.out b/qpdf/qtest/qpdf/issue-fuzz.out deleted file mode 100644 index 456485b..0000000 --- a/qpdf/qtest/qpdf/issue-fuzz.out +++ /dev/null @@ -1,19 +0,0 @@ -WARNING: issue-fuzz.pdf: can't find PDF header -WARNING: issue-fuzz.pdf (xref table, offset 19): accepting invalid xref table entry -WARNING: issue-fuzz.pdf (trailer, offset 36): unknown token while reading object; treating as string -WARNING: issue-fuzz.pdf (trailer, offset 53): unexpected > -WARNING: issue-fuzz.pdf (trailer, offset 54): unknown token while reading object; treating as string -WARNING: issue-fuzz.pdf (trailer, offset 58): unknown token while reading object; treating as string -WARNING: issue-fuzz.pdf (trailer, offset 72): unknown token while reading object; treating as string -WARNING: issue-fuzz.pdf (trailer, offset 36): dictionary ended prematurely; using null as value for last key -WARNING: issue-fuzz.pdf (trailer, offset 36): expected dictionary key but found non-name object; inserting key /QPDFFake1 -WARNING: issue-fuzz.pdf (trailer, offset 36): expected dictionary key but found non-name object; inserting key /QPDFFake2 -WARNING: issue-fuzz.pdf (trailer, offset 36): expected dictionary key but found non-name object; inserting key /QPDFFake3 -WARNING: issue-fuzz.pdf (trailer, offset 36): expected dictionary key but found non-name object; inserting key /QPDFFake4 -WARNING: issue-fuzz.pdf (trailer, offset 36): expected dictionary key but found non-name object; inserting key /QPDFFake5 -WARNING: issue-fuzz.pdf (trailer, offset 36): expected dictionary key but found non-name object; inserting key /QPDFFake6 -WARNING: issue-fuzz.pdf (trailer, offset 36): expected dictionary key but found non-name object; inserting key /QPDFFake7 -WARNING: issue-fuzz.pdf: file is damaged -WARNING: issue-fuzz.pdf (trailer, offset 32): /Size key in trailer dictionary is impossibly large -WARNING: issue-fuzz.pdf: Attempting to reconstruct cross-reference table -qpdf: issue-fuzz.pdf: unable to find /Root dictionary diff --git a/qpdf/qtest/qpdf/issue-fuzz.pdf b/qpdf/qtest/qpdf/issue-fuzz.pdf deleted file mode 100644 index 288a6b5..0000000 Binary files a/qpdf/qtest/qpdf/issue-fuzz.pdf and /dev/null differ diff --git a/qpdf/qtest/specific-bugs.test b/qpdf/qtest/specific-bugs.test index 428471b..99a7e80 100644 --- a/qpdf/qtest/specific-bugs.test +++ b/qpdf/qtest/specific-bugs.test @@ -38,7 +38,6 @@ my @bug_tests = ( ["263", "empty xref stream", 2], ["335a", "ozz-fuzz-12152", 2], ["335b", "ozz-fuzz-14845", 2], - ["fuzz", "impossibly large trailer /Size"], # ["fuzz-16214", "stream in object stream", 3, "--preserve-unreferenced"], # When adding to this list, consider adding to CORPUS_FROM_TEST in # fuzz/CMakeLists.txt and updating the count in -- libgit2 0.21.4