Merge pull request #1258 from m-holger/fuzz

Adjust fuzzer memory limits and refactor Pl_RunLength::decode

fuzz/CMakeLists.txt

@@ -100,6 +100,7 @@ set(CORPUS_OTHER
   16953.fuzz
   17630.fuzz
   17630a.fuzz
+  17630b.fuzz
   18241.fuzz
   18247.fuzz
   23172.fuzz
@@ -128,6 +129,7 @@ set(CORPUS_OTHER
   69977b.fuzz
   69977c.fuzz
   69977d.fuzz
+  69977e.fuzz
   70055.fuzz
   70245.fuzz
   70306.fuzz

fuzz/dct_fuzzer.cc

@@ -30,7 +30,7 @@ FuzzHelper::doChecks()
     // fuzzing is due to corrupt JPEG data which sometimes cannot be detected before
     // jpeg_start_decompress is called. During normal use of qpdf very large JPEGs can occasionally
     // occur legitimately and therefore must be allowed during normal operations.
-    Pl_DCT::setMemoryLimit(1'000'000'000);
+    Pl_DCT::setMemoryLimit(200'000'000);
 
     // Do not decompress corrupt data. This may cause extended runtime within jpeglib without
     // exercising additional code paths in qpdf.

fuzz/qpdf_fuzzer.cc

@@ -180,11 +180,11 @@ FuzzHelper::doChecks()
     // fuzzing is due to corrupt JPEG data which sometimes cannot be detected before
     // jpeg_start_decompress is called. During normal use of qpdf very large JPEGs can occasionally
     // occur legitimately and therefore must be allowed during normal operations.
-    Pl_DCT::setMemoryLimit(1'000'000'000);
+    Pl_DCT::setMemoryLimit(100'000'000);
 
     Pl_PNGFilter::setMemoryLimit(1'000'000);
     Pl_TIFFPredictor::setMemoryLimit(1'000'000);
-    Pl_Flate::setMemoryLimit(10'000'000);
+    Pl_Flate::setMemoryLimit(1'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' => 73],             # increment when adding new files
+    ['qpdf' => 75],             # increment when adding new files
     );
 
 my $n_tests = 0;

include/qpdf/Pl_RunLength.hh

@@ -62,6 +62,7 @@ class QPDF_DLL_CLASS Pl_RunLength: public Pipeline
         state_e state;
         unsigned char buf[128];
         unsigned int length;
+        std::string out;
     };
 
     std::shared_ptr<Members> m;

libqpdf/Pl_RunLength.cc

@@ -66,8 +66,9 @@ Pl_RunLength::encode(unsigned char const* data, size_t len)
 void
 Pl_RunLength::decode(unsigned char const* data, size_t len)
 {
+    m->out.reserve(len);
     for (size_t i = 0; i < len; ++i) {
-        unsigned char ch = data[i];
+        unsigned char const& ch = data[i];
         switch (m->state) {
         case st_top:
             if (ch < 128) {
@@ -85,16 +86,14 @@ Pl_RunLength::decode(unsigned char const* data, size_t len)
             break;
 
         case st_copying:
-            this->getNext()->write(&ch, 1);
+            m->out.append(1, static_cast<char>(ch));
             if (--m->length == 0) {
                 m->state = st_top;
             }
             break;
 
         case st_run:
-            for (unsigned int j = 0; j < m->length; ++j) {
+            m->out.append(m->length, static_cast<char>(ch));
-                this->getNext()->write(&ch, 1);
-            }
             m->state = st_top;
             break;
         }
@@ -137,10 +136,13 @@ Pl_RunLength::finish()
     // When decoding, we might have read a length byte not followed by data, which means the stream
     // was terminated early, but we will just ignore this case since this is the only sensible thing
     // to do.
+    auto next = getNext();
     if (m->action == a_encode) {
         flush_encode();
         unsigned char ch = 128;
-        this->getNext()->write(&ch, 1);
+        next->write(&ch, 1);
+    } else {
+        next->writeString(m->out);
     }
-    this->getNext()->finish();
+    next->finish();
 }