Merge pull request #1289 from m-holger/fuzz

Fix bugs found during fuzzing

fuzz/CMakeLists.txt

@@ -142,6 +142,9 @@ set(CORPUS_OTHER
   70306b.fuzz
   71624.fuzz
   71689.fuzz
+  99999a.fuzz
+  99999b.fuzz
+  99999c.fuzz
 )
 
 set(CORPUS_DIR ${CMAKE_CURRENT_BINARY_DIR}/qpdf_corpus)

fuzz/qpdf_extra/99999a.fuzz

@@ -0,0 +1,63 @@
+%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

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 = 79;       # increment when adding new files
+my $n_qpdf_files = 82;       # increment when adding new files
 
 my @fuzzers = (
     ['ascii85' => 1],

libqpdf/QPDF.cc

@@ -832,10 +832,6 @@ std::vector<QPDF::Xref_table::Subsection>
 QPDF::Xref_table::bad_subsections(std::string& line, qpdf_offset_t start)
 {
     std::vector<QPDF::Xref_table::Subsection> result;
-    qpdf_offset_t f1 = 0;
-    int f2 = 0;
-    char type = '\0';
-
     file->seek(start, SEEK_SET);
 
     while (true) {
@@ -844,7 +840,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 (!read_entry(f1, f2, type)) {
+            if (!std::get<0>(read_entry())) {
                 QTC::TC("qpdf", "QPDF invalid xref entry");
                 throw damaged_table("invalid xref entry (obj=" + std::to_string(i) + ")");
             }
@@ -890,9 +886,13 @@ QPDF::Xref_table::subsections(std::string& line)
     }
 }
 
-bool
+// Returns (success, f1, f2, type).
-QPDF::Xref_table::read_bad_entry(qpdf_offset_t& f1, int& f2, char& type)
+std::tuple<bool, qpdf_offset_t, int, char>
+QPDF::Xref_table::read_bad_entry()
 {
+    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(qpdf_offset_t& f1, int& f2, char& type)
     }
     // Require digit
     if (!QUtil::is_digit(*p)) {
-        return false;
+        return {false, 0, 0, '\0'};
     }
     // Gather digits
     std::string f1_str;
@@ -919,7 +919,7 @@ QPDF::Xref_table::read_bad_entry(qpdf_offset_t& f1, int& f2, char& type)
     }
     // Require space
     if (!QUtil::is_space(*p)) {
-        return false;
+        return {false, 0, 0, '\0'};
     }
     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(qpdf_offset_t& f1, int& f2, char& type)
     }
     // Require digit
     if (!QUtil::is_digit(*p)) {
-        return false;
+        return {false, 0, 0, '\0'};
     }
     // Gather digits
     std::string f2_str;
@@ -940,7 +940,7 @@ QPDF::Xref_table::read_bad_entry(qpdf_offset_t& f1, int& f2, char& type)
     }
     // Require space
     if (!QUtil::is_space(*p)) {
-        return false;
+        return {false, 0, 0, '\0'};
     }
     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(qpdf_offset_t& f1, int& f2, char& type)
     if ((*p == 'f') || (*p == 'n')) {
         type = *p;
     } else {
-        return false;
+        return {false, 0, 0, '\0'};
     }
     if ((f1_str.length() != 10) || (f2_str.length() != 5)) {
         QTC::TC("qpdf", "QPDF ignore length error xref entry");
@@ -967,18 +967,23 @@ QPDF::Xref_table::read_bad_entry(qpdf_offset_t& f1, int& f2, char& type)
     f1 = QUtil::string_to_ll(f1_str.c_str());
     f2 = QUtil::string_to_int(f2_str.c_str());
 
-    return true;
+    return {true, f1, f2, type};
 }
 
 // Optimistically read and parse xref entry. If entry is bad, call read_bad_xrefEntry and return
-// result.
+// result. Returns (success, f1, f2, type).
-bool
+std::tuple<bool, qpdf_offset_t, int, char>
-QPDF::Xref_table::read_entry(qpdf_offset_t& f1, int& f2, char& type)
+QPDF::Xref_table::read_entry()
 {
+    qpdf_offset_t f1{0};
+    int f2{0};
+    char type{'\0'};
     std::array<char, 21> line;
+    f1 = 0;
+    f2 = 0;
     if (file->read(line.data(), 20) != 20) {
         // C++20: [[unlikely]]
-        return false;
+        return {false, 0, 0, '\0'};
     }
     line[20] = '\0';
     char const* p = line.data();
@@ -1002,7 +1007,7 @@ QPDF::Xref_table::read_entry(qpdf_offset_t& f1, int& f2, char& type)
     if (!QUtil::is_space(*p++)) {
         // Entry doesn't start with space or digit.
         // C++20: [[unlikely]]
-        return false;
+        return {false, 0, 0, '\0'};
     }
     // Gather digits. NB No risk of overflow as 99'999 < max int.
     while (*p == '0') {
@@ -1019,10 +1024,10 @@ QPDF::Xref_table::read_entry(qpdf_offset_t& f1, int& f2, char& type)
         // No test for valid line[19].
         if (*(++p) && *(++p) && (*p == '\n' || *p == '\r') && f1_len == 10 && f2_len == 5) {
             // C++20: [[likely]]
-            return true;
+            return {true, f1, f2, type};
         }
     }
-    return read_bad_entry(f1, f2, type);
+    return read_bad_entry();
 }
 
 // Read a single cross-reference table section and associated trailer.
@@ -1052,7 +1057,10 @@ 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<unsigned int>(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);
     }
 
@@ -1064,10 +1072,8 @@ 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
-            qpdf_offset_t f1 = 0;
+            auto [success, f1, f2, type] = read_entry();
-            int f2 = 0;
+            if (!success) {
-            char type = '\0';
-            if (!read_entry(f1, f2, type)) {
                 throw damaged_table("invalid xref entry (obj=" + std::to_string(i) + ")");
             }
             if (type == 'f') {
@@ -1585,8 +1591,7 @@ QPDF::Xref_table::read_trailer()
 {
     qpdf_offset_t offset = file->tell();
     bool empty = false;
-    auto object =
+    auto object = QPDFParser(*file, "trailer", tokenizer, nullptr, &qpdf, true).parse(empty, false);
-        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.

libqpdf/qpdf/QPDF_private.hh

@@ -292,8 +292,8 @@ class QPDF::Xref_table
     std::vector<Subsection> subsections(std::string& line);
     std::vector<Subsection> bad_subsections(std::string& line, qpdf_offset_t offset);
     Subsection subsection(std::string const& line);
-    bool read_entry(qpdf_offset_t& f1, int& f2, char& type);
+    std::tuple<bool, qpdf_offset_t, int, char> read_entry();
-    bool read_bad_entry(qpdf_offset_t& f1, int& f2, char& type);
+    std::tuple<bool, qpdf_offset_t, int, char> read_bad_entry();
 
     // Methods to parse streams
     qpdf_offset_t read_stream(qpdf_offset_t offset);

qpdf/qpdf.testcov

@@ -55,6 +55,7 @@ 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

qpdf/qtest/qpdf/issue-fuzz.out

@@ -0,0 +1,19 @@
+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

qpdf/qtest/specific-bugs.test

@@ -38,6 +38,7 @@ 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