Merge pull request #1255 from m-holger/fuzz

Refactor xref reconstruction

fuzz/CMakeLists.txt

@@ -127,6 +127,7 @@ set(CORPUS_OTHER
   69977a.fuzz
   69977b.fuzz
   69977c.fuzz
+  69977d.fuzz
   70055.fuzz
   70245.fuzz
   70306.fuzz

fuzz/qpdf_fuzzer.cc

@@ -2,6 +2,7 @@
 #include <qpdf/BufferInputSource.hh>
 #include <qpdf/Pl_DCT.hh>
 #include <qpdf/Pl_Discard.hh>
+#include <qpdf/Pl_Flate.hh>
 #include <qpdf/Pl_PNGFilter.hh>
 #include <qpdf/Pl_TIFFPredictor.hh>
 #include <qpdf/QPDF.hh>
@@ -183,6 +184,7 @@ FuzzHelper::doChecks()
 
     Pl_PNGFilter::setMemoryLimit(1'000'000);
     Pl_TIFFPredictor::setMemoryLimit(1'000'000);
+    Pl_Flate::setMemoryLimit(10'000'000);
 
     // Do not decompress corrupt data. This may cause extended runtime within jpeglib without
     // exercising additional code paths in qpdf, and potentially causing counterproductive timeouts.

fuzz/qtest/fuzz.test

@@ -21,7 +21,7 @@ my @fuzzers = (
     ['pngpredictor' => 1],
     ['runlength' => 6],
     ['tiffpredictor' => 2],
-    ['qpdf' => 72],             # increment when adding new files
+    ['qpdf' => 73],             # increment when adding new files
     );
 
 my $n_tests = 0;

include/qpdf/Pl_Flate.hh

@@ -42,6 +42,11 @@ class QPDF_DLL_CLASS Pl_Flate: public Pipeline
     QPDF_DLL
     ~Pl_Flate() override;
 
+    // Limit the memory used.
+    // NB This is a static option affecting all Pl_PNGFilter instances.
+    QPDF_DLL
+    static void setMemoryLimit(unsigned long long limit);
+
     QPDF_DLL
     void write(unsigned char const* data, size_t len) override;
     QPDF_DLL
@@ -87,6 +92,7 @@ class QPDF_DLL_CLASS Pl_Flate: public Pipeline
         action_e action;
         bool initialized;
         void* zdata;
+        unsigned long long written{0};
         std::function<void(char const*, int)> callback;
     };
 

libqpdf/Pl_Flate.cc

@@ -7,6 +7,11 @@
 #include <qpdf/QIntC.hh>
 #include <qpdf/QUtil.hh>
 
+namespace
+{
+    unsigned long long memory_limit{0};
+} // namespace
+
 int Pl_Flate::compression_level = Z_DEFAULT_COMPRESSION;
 
 Pl_Flate::Members::Members(size_t out_bufsize, action_e action) :
@@ -63,6 +68,12 @@ Pl_Flate::~Pl_Flate() // NOLINT (modernize-use-equals-default)
     // Must be explicit and not inline -- see QPDF_DLL_CLASS in README-maintainer
 }
 
+void
+Pl_Flate::setMemoryLimit(unsigned long long limit)
+{
+    memory_limit = limit;
+}
+
 void
 Pl_Flate::setWarnCallback(std::function<void(char const*, int)> callback)
 {
@@ -170,6 +181,12 @@ Pl_Flate::handleData(unsigned char const* data, size_t len, int flush)
                 }
                 uLong ready = QIntC::to_ulong(m->out_bufsize - zstream.avail_out);
                 if (ready > 0) {
+                    if (memory_limit) {
+                        m->written += ready;
+                        if (m->written > memory_limit) {
+                            throw std::runtime_error("PL_Flate memory limit exceeded");
+                        }
+                    }
                     this->getNext()->write(m->outbuf.get(), ready);
                     zstream.next_out = m->outbuf.get();
                     zstream.avail_out = QIntC::to_uint(m->out_bufsize);

libqpdf/QPDF.cc

@@ -572,18 +572,13 @@ QPDF::reconstruct_xref(QPDFExc& e)
     m->file->seek(0, SEEK_END);
     qpdf_offset_t eof = m->file->tell();
     m->file->seek(0, SEEK_SET);
-    qpdf_offset_t line_start = 0;
-    // Don't allow very long tokens here during recovery.
-    static size_t const MAX_LEN = 100;
+    // Don't allow very long tokens here during recovery. All the interesting tokens are covered.
+    static size_t const MAX_LEN = 10;
     while (m->file->tell() < eof) {
-        m->file->findAndSkipNextEOL();
-        qpdf_offset_t next_line_start = m->file->tell();
-        m->file->seek(line_start, SEEK_SET);
         QPDFTokenizer::Token t1 = readToken(m->file, MAX_LEN);
         qpdf_offset_t token_start = m->file->tell() - toO(t1.getValue().length());
-        if (token_start >= next_line_start) {
-            // don't process yet -- wait until we get to the line containing this token
-        } else if (t1.isInteger()) {
+        if (t1.isInteger()) {
+            auto pos = m->file->tell();
             QPDFTokenizer::Token t2 = readToken(m->file, MAX_LEN);
             if ((t2.isInteger()) && (readToken(m->file, MAX_LEN).isWord("obj"))) {
                 int obj = QUtil::string_to_int(t1.getValue().c_str());
@@ -595,17 +590,19 @@ QPDF::reconstruct_xref(QPDFExc& e)
                         "", 0, "ignoring object with impossibly large id " + std::to_string(obj)));
                 }
             }
+            m->file->seek(pos, SEEK_SET);
         } else if (!m->trailer.isInitialized() && t1.isWord("trailer")) {
+            auto pos = m->file->tell();
             QPDFObjectHandle t = readTrailer();
             if (!t.isDictionary()) {
                 // Oh well.  It was worth a try.
             } else {
                 setTrailer(t);
             }
+            m->file->seek(pos, SEEK_SET);
         }
         check_warnings();
-        m->file->seek(next_line_start, SEEK_SET);
-        line_start = next_line_start;
+        m->file->findAndSkipNextEOL();
     }
     m->deleted_objects.clear();