Obscure bug fix copying foreign streams in special cases (fixes qpdf#449

)

Specifically, if a stream had its stream data replaced and had
indirect /Filter or /DecodeParms, it would result in non-silent loss
of data and/or internal error.

ChangeLog

@@ -1,5 +1,11 @@
 2020-10-21  Jay Berkenbilt  <[email protected]>
 
+	* Bug fix: properly handle copying foreign streams that have
+	indirect /Filter or /DecodeParms keys when stream data has been
+	replaced. The circumstances leading to this bug are very unusual
+	but would cause qpdf to either generate an internal error or some
+	other kind of warning situation if it would occur. Fixes #449.
+
 	* Qpdf's build and CI has been migrated from Azure Pipelines
 	(Azure Devops) to GitHub Actions.
 

TODO

@@ -7,7 +7,6 @@ Candidates for upcoming release
 * Open "next" issues
   * bugs
     * #473: zsh completion with directories
-    * #449: internal error with case to reproduce (from pikepdf)
     * #444: concatenated stream/whitespace bug
   * Non-bugs
     * #446: recognize edited QDF files

libqpdf/QPDF.cc

@@ -2313,7 +2313,8 @@ QPDF::reserveObjects(QPDFObjectHandle foreign, ObjCopier& obj_copier,
     if (foreign.isReserved())
     {
         throw std::logic_error(
-            "QPDF: attempting to copy a foreign reserved object");
+            "QPDF: attempting to copy a foreign reserved object: " +
+            QUtil::int_to_string(foreign.getObjectID()));
     }
 
     if (foreign.isPagesObject())
@@ -2486,6 +2487,16 @@ QPDF::replaceForeignIndirectObjects(
         }
         PointerHolder<Buffer> stream_buffer =
             stream->getStreamDataBuffer();
+        // Note: at this stage, dictionary keys may still be reserved.
+        // We have to handle that explicitly if we access anything.
+        auto get_as_direct = [&old_dict] (std::string const& key) {
+            QPDFObjectHandle obj = old_dict.getKey(key);
+            obj.makeDirect();
+            return obj;
+        };
+
+        QPDFObjectHandle filter = get_as_direct("/Filter");
+        QPDFObjectHandle decode_parms = get_as_direct("/DecodeParms");
         if ((foreign_stream_qpdf->m->immediate_copy_from) &&
             (stream_buffer.getPointer() == 0))
         {
@@ -2495,18 +2506,15 @@ QPDF::replaceForeignIndirectObjects(
             // have to keep duplicating the memory.
             QTC::TC("qpdf", "QPDF immediate copy stream data");
             foreign.replaceStreamData(foreign.getRawStreamData(),
-                                      dict.getKey("/Filter"),
-                                      dict.getKey("/DecodeParms"));
+                                      filter, decode_parms);
             stream_buffer = stream->getStreamDataBuffer();
         }
         PointerHolder<QPDFObjectHandle::StreamDataProvider> stream_provider =
             stream->getStreamDataProvider();
         if (stream_buffer.getPointer())
         {
             QTC::TC("qpdf", "QPDF copy foreign stream with buffer");
-            result.replaceStreamData(stream_buffer,
-                                     dict.getKey("/Filter"),
-                                     dict.getKey("/DecodeParms"));
+            result.replaceStreamData(stream_buffer, filter, decode_parms);
         }
         else if (stream_provider.getPointer())
         {
@@ -2515,8 +2523,7 @@ QPDF::replaceForeignIndirectObjects(
             this->m->copied_stream_data_provider->registerForeignStream(
                 local_og, foreign);
             result.replaceStreamData(this->m->copied_streams,
-                                     dict.getKey("/Filter"),
-                                     dict.getKey("/DecodeParms"));
+                                     filter, decode_parms);
         }
         else
         {
@@ -2534,8 +2541,7 @@ QPDF::replaceForeignIndirectObjects(
             this->m->copied_stream_data_provider->registerForeignStream(
                 local_og, foreign_stream_data);
             result.replaceStreamData(this->m->copied_streams,
-                                     dict.getKey("/Filter"),
-                                     dict.getKey("/DecodeParms"));
+                                     filter, decode_parms);
         }
     }
     else

make/msvc.mk

@@ -66,7 +66,7 @@ endef
 # Usage: $(call makelib,objs,library,ldflags,libs,current,revision,age)
 define makelib
 	cl -nologo -O2 -Zi -Gy -EHsc -MD -LD -Fe$(basename $(2))$(shell expr $(5) - $(7)).dll $(1) \
-		-link -SUBSYSTEM:CONSOLE,5.01 -incremental:no \
+		-link -SUBSYSTEM:CONSOLE -incremental:no \
 		$(foreach L,$(subst -L,,$(3)),-LIBPATH:$(L)) \
 		$(foreach L,$(subst -l,,$(4)),$(L).lib)
 	if [ -f $(basename $(2))$(shell expr $(5) - $(7)).dll.manifest ]; then \
@@ -81,7 +81,7 @@ endef
 define makebin
 	cl -nologo -O2 -Zi -Gy -EHsc -MD $(1) \
 		$(if $(5),$(5),$(WINDOWS_MAIN_XLINK_FLAGS)) \
-		-link -SUBSYSTEM:CONSOLE,5.01 -incremental:no -OUT:$(2) \
+		-link -SUBSYSTEM:CONSOLE -incremental:no -OUT:$(2) \
 		$(foreach L,$(subst -L,,$(3)),-LIBPATH:$(L)) \
 		$(foreach L,$(subst -l,,$(4)),$(L).lib)
 	if [ -f $(2).manifest ]; then \

qpdf/qtest/qpdf.test

@@ -2417,7 +2417,7 @@ $td->runtest("check output",
 show_ntests();
 # ----------
 $td->notify("--- Copy Foreign Objects ---");
-$n_tests += 7;
+$n_tests += 10;
 
 foreach my $d ([25, 1], [26, 2], [27, 3])
 {
@@ -2438,6 +2438,19 @@ $td->runtest("copy objects error",
              {$td->FILE => "copy-foreign-objects-errors.out",
               $td->EXIT_STATUS => 0},
              $td->NORMALIZE_NEWLINES);
+
+$td->runtest("indirect filters",
+             {$td->COMMAND => "test_driver 69 indirect-filter.pdf"},
+             {$td->STRING => "test 69 done\n", $td->EXIT_STATUS => 0},
+             $td->NORMALIZE_NEWLINES);
+foreach my $i (0, 1)
+{
+    $td->runtest("check output",
+                 {$td->FILE => "auto-$i.pdf"},
+                 {$td->FILE => "indirect-filter-out-$i.pdf"});
+}
+
+
 show_ntests();
 # ----------
 $td->notify("--- Error Condition Tests ---");

qpdf/test_driver.cc

@@ -2195,6 +2195,22 @@ void runtest(int n, char const* filename1, char const* arg2)
 	    std::cout << "raw stream data okay" << std::endl;
 	}
     }
+    else if (n == 69)
+    {
+        pdf.setImmediateCopyFrom(true);
+        auto pages = pdf.getAllPages();
+        for (size_t i = 0; i < pages.size(); ++i)
+        {
+            QPDF out;
+            out.emptyPDF();
+            out.addPage(pages.at(i), false);
+            std::string outname = std::string("auto-") +
+                QUtil::uint_to_string(i) + ".pdf";
+            QPDFWriter w(out, outname.c_str());
+            w.setStaticID(true);
+            w.write();
+        }
+    }
     else
     {
 	throw std::runtime_error(std::string("invalid test ") +