fix: validate PDF cross-reference targets before caching objects

CHANGELOG.md

@@ -16,6 +16,9 @@ The release run heads these entries with the version and opens a fresh
 
 ## Unreleased
 
+- PDF object reads reject mismatched cross-reference targets and recover
+  from a stream error before seeking to another object.
+
 - PDF incremental updates keep deleted and replaced objects from resolving
   through older generations. Hybrid tables discard superseded free entries.
 

src/odr/internal/pdf/AGENTS.md

@@ -61,6 +61,9 @@ output.
   `std::streambuf`. Random access seeks, sequential tokenizing peeks one char.
   An object parses when referenced and is cached by reference. Positions are
   `std::uint32_t`, so a file of 4 GiB or more is out of scope.
+  Parsers live for one render. Their caches retain whole decoded object streams,
+  so reading another member does not inflate or parse that stream again.
+  Object IDs and generations must match the cross-reference entry.
 - **Fail early on malformed structure, tolerate unknown content.** A missing
   `obj`, `endobj`, `stream`, `endstream`, `xref` or `startxref`, an unexpected
   char, an unknown page-tree type or an exhausted stream throws

src/odr/internal/pdf/pdf_document_parser.cpp

@@ -1637,8 +1637,13 @@ DocumentParser::read_object(const ObjectReference &reference) {
                                          << " not in cross-reference table, "
                                             "treating as null");
   } else if (const Xref::Entry &entry = entry_it->second; entry.is_used()) {
+    in().clear();
     in().seekg(entry.as_used().position);
     object = parser().read_indirect_object();
+    if (object.reference != reference) {
+      throw std::runtime_error(
+          "PDF object reference does not match its xref entry");
+    }
     // Decrypt string leaves (7.6.2). The /Encrypt dictionary needs no special
     // case here: it is read and cached during construction before the
     // decryptor is installed, so its un-decrypted /O,/U,… strings are served
@@ -1657,10 +1662,8 @@ DocumentParser::read_object(const ObjectReference &reference) {
       throw std::runtime_error("object stream member index out of range");
     }
     if (members[index].id != reference.id) {
-      ODR_WARNING(m_logger, "pdf: object stream "
-                                << stream_id << " member " << index
-                                << " has id " << members[index].id
-                                << ", expected " << reference.id);
+      throw std::runtime_error(
+          "PDF object stream member does not match its xref entry");
     }
     object.object = members[index].object;
   } else {
@@ -1731,6 +1734,7 @@ std::string DocumentParser::read_object_stream(const IndirectObject &object) {
   Object length = object.object.as_dictionary().get("Length");
   resolve_object(length);
 
+  in().clear();
   // a stream object always carries a stream position
   // NOLINTNEXTLINE(bugprone-unchecked-optional-access)
   in().seekg(object.stream_position.value());

src/odr/internal/pdf/pdf_document_parser.hpp

@@ -23,26 +23,11 @@ namespace odr::internal::pdf {
 
 struct Document;
 
-/// Resolution/memoization layer on top of the sequential `FileParser`: maps
-/// `ObjectReference`s to file positions via the cross-reference table and hands
-/// out resolved objects by reference.
-///
-/// The caches are intrinsic, not a convenience: reading one compressed object
-/// means inflating and parsing the whole object stream holding it, and only
-/// this class knows from the xref which objects share one. They grow
-/// monotonically and are never evicted, which is safe only because a
-/// `DocumentParser` is transient per render — build one, produce a `Document`,
-/// read its lazy streams, discard it. Never share one across documents.
+/// Resolves and caches objects for one PDF; construct a parser per render.
 class DocumentParser {
 public:
-  /// Takes ownership of the input stream (move-only: the parser is the
-  /// top-level handle for reading a PDF, so the stream's lifetime is tied to
-  /// it). The constructor walks the trailer chain and, if the file declares an
-  /// `/Encrypt` dictionary, builds the `Authenticator`. Pass an already
-  /// authenticated `decryptor` (from a prior `authenticate()` / `decryptor()`)
-  /// to re-open an encrypted file without the password; otherwise pass
-  /// `std::nullopt` to parse an unencrypted file or to call `authenticate()`
-  /// later. The logger is borrowed and must outlive the parser.
+  /// Owns the input and accepts a decryptor from an authenticated parser.
+  /// Otherwise probes the empty password; authenticate() unlocks later reads.
   explicit DocumentParser(std::unique_ptr<std::istream> in,
                           std::optional<Decryptor> decryptor = std::nullopt,
                           const Logger &logger = Logger::null());

test/src/internal/pdf/pdf_document_parser.cpp

@@ -790,3 +790,45 @@ TEST(DocumentParser, deleted_objects_do_not_resolve_to_an_older_generation) {
   EXPECT_TRUE(parser.read_object(ObjectReference{3, 0}).object.is_null());
   EXPECT_TRUE(parser.read_object(ObjectReference{3, 1}).object.is_null());
 }
+
+TEST(DocumentParser, rejects_mismatched_indirect_object_references) {
+  for (const std::string header : {"9 0 obj", "1 1 obj"}) {
+    std::string source = two_object_mini_pdf(true);
+    source.replace(source.find("1 0 obj"), header.size(), header);
+    DocumentParser parser(std::make_unique<std::istringstream>(source));
+    ASSERT_FALSE(parser.is_recovered());
+    EXPECT_THROW((void)parser.read_object(ObjectReference{1, 0}),
+                 std::runtime_error);
+  }
+}
+
+TEST(DocumentParser, object_reads_reset_stream_errors) {
+  DocumentParser parser(
+      std::make_unique<std::istringstream>(two_object_mini_pdf(true)));
+  parser.in().setstate(std::ios::eofbit | std::ios::failbit);
+  EXPECT_EQ(parser.read_object(ObjectReference{1, 0})
+                .object.as_dictionary()
+                .get("Type")
+                .as_name(),
+            "Catalog");
+  for (std::uint32_t attempt = 0; attempt < 2; ++attempt) {
+    parser.in().setstate(std::ios::eofbit | std::ios::failbit);
+    EXPECT_EQ(parser.read_decoded_stream(ObjectReference{4, 0}), "BT ET");
+  }
+}
+
+TEST(DocumentParser, rejects_mismatched_object_stream_members) {
+  PdfFileBuilder builder;
+  builder.stream_object("/Type /ObjStm /N 1 /First 4", "9 0 (wrong)")
+      .object("null")
+      .trailer("/Root 2 0 R");
+  std::string source = builder.build_xref_stream();
+  const std::size_t start = source.find("stream\n", source.find("/Type /XRef"));
+  ASSERT_NE(start, std::string::npos);
+  // Entry 2: compressed in object stream 1, member 0 (builder uses W [1 4 2]).
+  source.replace(start + 7 + 2 * 7, 7, std::string("\x02\0\0\0\x01\0\0", 7));
+  DocumentParser parser(std::make_unique<std::istringstream>(source));
+  ASSERT_FALSE(parser.is_recovered());
+  EXPECT_THROW((void)parser.read_object(ObjectReference{2, 0}),
+               std::runtime_error);
+}