Merge pull request #1195 from m-holger/fuzz

Fix bug in QPDFWriter::preserveObjectStreams

Merge pull request #1195 from m-holger/fuzz
Fix bug in QPDFWriter::preserveObjectStreams
Jay Berkenbilt · GitHub
2 parents 6ae439f1 6f09069f
Showing 7 changed files with 37 additions and 34 deletions
fuzz/CMakeLists.txt
fuzz/qpdf_extra/68668.fuzz
fuzz/qpdf_extra/68915.fuzz
fuzz/qtest/fuzz.test
libqpdf/QPDFWriter.cc
libqpdf/QUtil.cc
qpdf/qpdf.testcov
@@ -116,6 +116,8 @@ set(CORPUS_OTHER
   65777.fuzz
   68374.fuzz
   68377.fuzz
+  68668.fuzz
+  68915.fuzz
 )
  
 set(CORPUS_DIR ${CMAKE_CURRENT_BINARY_DIR}/qpdf_corpus)
@@ -21,7 +21,7 @@ my @fuzzers = (
     ['pngpredictor' => 1],
     ['runlength' => 6],
     ['tiffpredictor' => 2],
-    ['qpdf' => 58],             # increment when adding new files
+    ['qpdf' => 60],             # increment when adding new files
     );
  
 my $n_tests = 0;
@@ -1944,28 +1944,30 @@ QPDFWriter::preserveObjectStreams()
     // that are not allowed to be in object streams. In addition to removing objects that were
     // erroneously included in object streams in the source PDF, it also prevents unreferenced
     // objects from being included.
-    auto iter = xref.cbegin();
     auto end = xref.cend();
-
-    // Start by scanning for first compressed object in case we don't have any object streams to
-    // process.
-    for (; iter != end; ++iter) {
-        if (iter->second.getType() == 2) {
-            // Pdf contains object streams.
-            QTC::TC(
-                "qpdf",
-                "QPDFWriter preserve object streams",
-                m->preserve_unreferenced_objects ? 0 : 1);
-
-            if (m->preserve_unreferenced_objects) {
-                for (; iter != end; ++iter) {
-                    if (iter->second.getType() == 2) {
-                        m->obj[iter->first].object_stream = iter->second.getObjStreamNumber();
-                    }
-                }
-            } else {
+    m->obj.streams_empty = true;
+    if (m->preserve_unreferenced_objects) {
+        for (auto iter = xref.cbegin(); iter != end; ++iter) {
+            if (iter->second.getType() == 2) {
+                // Pdf contains object streams.
+                QTC::TC("qpdf", "QPDFWriter preserve object streams preserve unreferenced");
+                m->obj.streams_empty = false;
+                m->obj[iter->first].object_stream = iter->second.getObjStreamNumber();
+            }
+        }
+    } else {
+        // Start by scanning for first compressed object in case we don't have any object streams to
+        // process.
+        for (auto iter = xref.cbegin(); iter != end; ++iter) {
+            if (iter->second.getType() == 2) {
+                // Pdf contains object streams.
+                QTC::TC("qpdf", "QPDFWriter preserve object streams");
+                m->obj.streams_empty = false;
                 auto eligible = QPDF::Writer::getCompressibleObjSet(m->pdf);
-                for (; iter != end; ++iter) {
+                // The object pointed to by iter may be a previous generation, in which case it is
+                // removed by getCompressibleObjSet. We need to restart the loop (while the object
+                // table may contain multiple generations of an object).
+                for (iter = xref.cbegin(); iter != end; ++iter) {
                     if (iter->second.getType() == 2) {
                         auto id = static_cast<size_t>(iter->first.getObj());
                         if (id < eligible.size() && eligible[id]) {
@@ -1975,12 +1977,10 @@ QPDFWriter::preserveObjectStreams()
                         }
                     }
                 }
+                return;
             }
-            return;
         }
     }
-    // No compressed objects found.
-    m->obj.streams_empty = true;
 }
  
 void
@@ -1899,7 +1899,8 @@ call_main_from_wmain(
     // strings for compatibility with other systems. That way the rest of qpdf.cc can just act like
     // arguments are UTF-8.
  
-    std::vector<std::unique_ptr<char[]>> utf8_argv;
+    std::vector<std::string> utf8_argv;
+    utf8_argv.reserve(QIntC::to_size(argc));
     for (int i = 0; i < argc; ++i) {
         std::string utf16;
         for (size_t j = 0; j < std::wcslen(argv[i]); ++j) {
@@ -1907,17 +1908,16 @@ call_main_from_wmain(
             utf16.append(1, static_cast<char>(QIntC::to_uchar(codepoint >> 8)));
             utf16.append(1, static_cast<char>(QIntC::to_uchar(codepoint & 0xff)));
         }
-        std::string utf8 = QUtil::utf16_to_utf8(utf16);
-        utf8_argv.push_back(QUtil::make_unique_cstr(utf8));
+        utf8_argv.emplace_back(QUtil::utf16_to_utf8(utf16));
     }
-    auto utf8_argv_sp = std::make_unique<char*[]>(1 + utf8_argv.size());
-    char** new_argv = utf8_argv_sp.get();
-    for (size_t i = 0; i < utf8_argv.size(); ++i) {
-        new_argv[i] = utf8_argv.at(i).get();
+    std::vector<char*> new_argv;
+    new_argv.reserve(utf8_argv.size() + 1U);
+    for (auto const& arg: utf8_argv) {
+        new_argv.emplace_back(const_cast<char*>(arg.data()));
     }
     argc = QIntC::to_int(utf8_argv.size());
-    new_argv[argc] = nullptr;
-    return realmain(argc, new_argv);
+    new_argv.emplace_back(nullptr);
+    return realmain(argc, new_argv.data());
 }
  
 int
@@ -600,7 +600,8 @@ QPDFAcroFormDocumentHelper AP parse error 0
 QPDFJob copy fields not this file 0
 QPDFJob copy fields non-first from orig 0
 QPDF resolve duplicated page in insert 0
-QPDFWriter preserve object streams 1
+QPDFWriter preserve object streams 0
+QPDFWriter preserve object streams preserve unreferenced 0
 QPDFWriter exclude from object stream 0
 QPDF_pages findPage not found 0
 QPDFObjectHandle check ownership 0