diff --git a/CHANGELOG.md b/CHANGELOG.md index 9e3f944a8..461cce89c 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/src/odr/internal/pdf/pdf_writer.cpp b/src/odr/internal/pdf/pdf_writer.cpp index 25e44b3f5..2339f1dbf 100644 --- a/src/odr/internal/pdf/pdf_writer.cpp +++ b/src/odr/internal/pdf/pdf_writer.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include @@ -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((placement.offset >> shift) & 0xff)); } result.push_back(static_cast((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(in.tellg()); + const auto size = checked_file_index(static_cast(in.tellg())); if (size == 0) { return {0, true}; } in.seekg(-1, std::ios::end); const char last = static_cast(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(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(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(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(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(update.size())); - in.clear(); - in.seekg(resume); + if (!out) { + throw std::ios_base::failure("cannot write PDF update"); + } + resume.restore(); } } // namespace odr::internal::pdf diff --git a/src/odr/internal/pdf/pdf_writer.hpp b/src/odr/internal/pdf/pdf_writer.hpp index 5738c2667..34f52ef51 100644 --- a/src/odr/internal/pdf/pdf_writer.hpp +++ b/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 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; diff --git a/test/src/internal/pdf/pdf_writer.cpp b/test/src/internal/pdf/pdf_writer.cpp index 48040adf9..688b68dd1 100644 --- a/test/src/internal/pdf/pdf_writer.cpp +++ b/test/src/internal/pdf/pdf_writer.cpp @@ -9,6 +9,7 @@ #include +#include #include #include #include @@ -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(count)); + remaining -= written; + return static_cast(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()); + } +}