fix(pdf): validate incremental writes and restore input state

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- PDF incremental writes reject invalid or conflicting object references and
+  oversized output, report write failures and restore the parser position.
+
 - PDF Lab and ICC colors, indexed palettes and image samples honor component
   ranges. Invalid Lab white points and malformed range arrays are rejected.
 

src/odr/internal/pdf/pdf_writer.cpp

@@ -2,6 +2,7 @@
 
 #include <odr/internal/crypto/crypto_util.hpp>
 #include <odr/internal/pdf/pdf_document_parser.hpp>
+#include <odr/internal/pdf/pdf_file_object.hpp>
 #include <odr/internal/util/stream_util.hpp>
 
 #include <algorithm>
@@ -47,7 +48,7 @@ std::string xref_stream_table(const Placements &placements) {
   std::string result;
   for (const auto &[id, placement] : placements) {
     result.push_back(1);
-    for (int shift = 24; shift >= 0; shift -= 8) {
+    for (std::int32_t shift = 24; shift >= 0; shift -= 8) {
       result.push_back(static_cast<char>((placement.offset >> shift) & 0xff));
     }
     result.push_back(static_cast<char>((placement.gen >> 8) & 0xff));
@@ -56,6 +57,42 @@ std::string xref_stream_table(const Placements &placements) {
   return result;
 }
 
+class InputPosition final {
+public:
+  explicit InputPosition(std::istream &in) : m_in{&in} {
+    m_in->clear();
+    m_position = m_in->tellg();
+    if (m_position == std::streampos{-1}) {
+      throw std::ios_base::failure("cannot save PDF input position");
+    }
+  }
+
+  ~InputPosition() {
+    try {
+      restore();
+    } catch (...) { // NOLINT(bugprone-empty-catch)
+      // Preserve the original exception if restoring also fails.
+    }
+  }
+
+  void restore() {
+    if (m_restored) {
+      return;
+    }
+    m_in->clear();
+    m_in->seekg(m_position);
+    if (!*m_in) {
+      throw std::ios_base::failure("cannot restore PDF input position");
+    }
+    m_restored = true;
+  }
+
+private:
+  std::istream *m_in{nullptr};
+  std::streampos m_position;
+  bool m_restored{false};
+};
+
 struct SourceExtent {
   std::uint32_t size{0};
   bool ends_with_eol{false};
@@ -64,12 +101,15 @@ struct SourceExtent {
 SourceExtent measure(std::istream &in) {
   in.clear();
   in.seekg(0, std::ios::end);
-  const auto size = static_cast<std::uint32_t>(in.tellg());
+  const auto size = checked_file_index(static_cast<std::streamoff>(in.tellg()));
   if (size == 0) {
     return {0, true};
   }
   in.seekg(-1, std::ios::end);
   const char last = static_cast<char>(in.get());
+  if (!in) {
+    throw std::ios_base::failure("cannot read PDF source");
+  }
   return {size, last == '\n' || last == '\r'};
 }
 
@@ -95,21 +135,35 @@ IncrementalWriter::IncrementalWriter(DocumentParser &parser)
 }
 
 ObjectReference IncrementalWriter::mint_object() {
+  checked_file_index(m_next_id, UINT32_MAX - 1);
   return ObjectReference(m_next_id++, 0);
 }
 
 void IncrementalWriter::set_object(const ObjectReference &reference,
                                    Object object) {
-  m_entries[reference] = Entry{std::move(object), std::nullopt};
-  m_next_id = std::max(m_next_id, reference.id + 1);
+  set_entry(reference, Entry{std::move(object), std::nullopt});
 }
 
 void IncrementalWriter::set_stream_object(const ObjectReference &reference,
                                           Dictionary dictionary,
                                           std::string stream) {
-  dictionary["Length"] = Object(static_cast<Integer>(stream.size()));
-  m_entries[reference] =
-      Entry{Object(std::move(dictionary)), std::move(stream)};
+  dictionary["Length"] = Object(Integer{checked_file_index(stream.size())});
+  set_entry(reference, Entry{Object(std::move(dictionary)), std::move(stream)});
+}
+
+void IncrementalWriter::set_entry(const ObjectReference &reference,
+                                  Entry entry) {
+  checked_file_index(reference.id, UINT32_MAX - 1);
+  checked_file_index(reference.gen, UINT16_MAX);
+  if (reference.id == 0) {
+    throw std::runtime_error("PDF object zero is reserved");
+  }
+  const auto existing = m_entries.lower_bound(ObjectReference{reference.id, 0});
+  if (existing != m_entries.end() && existing->first.id == reference.id &&
+      existing->first.gen != reference.gen) {
+    throw std::runtime_error("conflicting PDF object generations");
+  }
+  m_entries[reference] = std::move(entry);
   m_next_id = std::max(m_next_id, reference.id + 1);
 }
 
@@ -119,7 +173,7 @@ IncrementalWriter::build_trailer(const std::uint64_t size,
   const Dictionary &source = m_parser->trailer();
 
   Dictionary result;
-  result["Size"] = Object(static_cast<Integer>(size));
+  result["Size"] = Object(Integer{checked_file_index(size)});
   result["Root"] = source.get("Root");
   if (source.has_value("Info")) {
     result["Info"] = source.get("Info");
@@ -143,7 +197,7 @@ IncrementalWriter::build_trailer(const std::uint64_t size,
 
 void IncrementalWriter::write(std::ostream &out) const {
   std::istream &in = m_parser->in();
-  const std::streampos resume = in.tellg();
+  InputPosition resume(in);
 
   const SourceExtent source = measure(in);
   // An object must start on its own line; a `%%EOF` may end the file bare.
@@ -152,7 +206,7 @@ void IncrementalWriter::write(std::ostream &out) const {
   // Only the update is buffered; the source is piped.
   std::string update = separator;
   const auto position = [&] {
-    return static_cast<std::uint32_t>(source.size + update.size());
+    return checked_file_index(std::uint64_t{source.size} + update.size());
   };
 
   Placements placements;
@@ -223,13 +277,20 @@ void IncrementalWriter::write(std::ostream &out) const {
 
   update += fmt::format("startxref\n{}\n%%EOF\n", xref_position);
 
+  // the finished file has to fit the 32-bit offsets its xref states
+  (void)position();
+  if (!std::in_range<std::streamsize>(update.size())) {
+    throw std::length_error("PDF update is too large");
+  }
   in.clear();
   in.seekg(0);
   util::stream::pipe(in, out);
   out.write(update.data(), static_cast<std::streamsize>(update.size()));
 
-  in.clear();
-  in.seekg(resume);
+  if (!out) {
+    throw std::ios_base::failure("cannot write PDF update");
+  }
+  resume.restore();
 }
 
 } // namespace odr::internal::pdf

src/odr/internal/pdf/pdf_writer.hpp

@@ -35,7 +35,7 @@ class IncrementalWriter final {
 
   [[nodiscard]] std::size_t size() const noexcept { return m_entries.size(); }
 
-  /// Legal with nothing set: the result parses identically.
+  /// Copy and append, restoring the parser position even on failure.
   void write(std::ostream &out) const;
 
 private:
@@ -44,6 +44,8 @@ class IncrementalWriter final {
     std::optional<std::string> stream;
   };
 
+  void set_entry(const ObjectReference &reference, Entry entry);
+
   /// `/Root`, `/Info` and `/ID` from the source, plus `/Size` and `/Prev`.
   [[nodiscard]] Dictionary build_trailer(std::uint64_t size,
                                          std::string_view revision_seed) const;

test/src/internal/pdf/pdf_writer.cpp

@@ -9,6 +9,7 @@
 
 #include <internal/pdf/pdf_test_file_builder.hpp>
 
+#include <algorithm>
 #include <memory>
 #include <sstream>
 #include <string>
@@ -291,3 +292,60 @@ TEST(IncrementalWriter, rewrites_a_page_out_of_an_object_stream) {
     }
   }
 }
+
+TEST(IncrementalWriter, rejects_invalid_or_conflicting_references) {
+  DocumentParser parser(stream_of(mini_pdf(true)));
+  IncrementalWriter writer(parser);
+  for (const ObjectReference reference :
+       {ObjectReference{0, 0}, ObjectReference{0x100000000ULL, 0},
+        ObjectReference{0xffffffff, 0}, ObjectReference{1, 65536}}) {
+    EXPECT_THROW(writer.set_object(reference, Object(Integer{1})),
+                 std::runtime_error);
+    EXPECT_THROW(writer.set_stream_object(reference, Dictionary{}, ""),
+                 std::runtime_error);
+  }
+  EXPECT_EQ(writer.size(), 0u);
+  writer.set_object({5, 0}, Object(Integer{1}));
+  EXPECT_THROW(writer.set_object({5, 1}, Object(Integer{2})),
+               std::runtime_error);
+  EXPECT_EQ(writer.size(), 1u);
+  writer.set_object({5, 0}, Object(Integer{3}));
+  EXPECT_EQ(writer.size(), 1u);
+  writer.set_object({0xfffffffe, 65535}, Object(Integer{4}));
+  EXPECT_THROW((void)writer.mint_object(), std::runtime_error);
+}
+
+TEST(IncrementalWriter, write_failures_restore_the_parser_position) {
+  class LimitedBuffer final : public std::streambuf {
+  public:
+    explicit LimitedBuffer(const std::size_t limit) : remaining(limit) {}
+    std::streamsize xsputn(const char *, const std::streamsize count) override {
+      const auto written = std::min(remaining, static_cast<std::size_t>(count));
+      remaining -= written;
+      return static_cast<std::streamsize>(written);
+    }
+
+  private:
+    std::size_t remaining;
+  };
+  const std::string source = mini_pdf(true);
+  for (const std::size_t limit :
+       {std::size_t{0}, source.size(), source.size() + 5}) {
+    SCOPED_TRACE(limit);
+    DocumentParser parser(stream_of(source));
+    IncrementalWriter writer(parser);
+    parser.in().clear();
+    parser.in().seekg(7);
+    LimitedBuffer buffer(limit);
+    std::ostream failed(&buffer);
+    EXPECT_THROW(writer.write(failed), std::ios_base::failure);
+    EXPECT_EQ(parser.in().tellg(), std::streampos{7});
+
+    parser.in().setstate(std::ios::eofbit);
+    std::ostringstream good;
+    EXPECT_NO_THROW(writer.write(good));
+    EXPECT_EQ(parser.in().tellg(), std::streampos{7});
+    DocumentParser written(stream_of(good.str()));
+    EXPECT_FALSE(written.is_recovered());
+  }
+}