Harden PdfParser: bound serialization recursion and reject dangling object references

Tests/test_pdfparser.py

@@ -7,8 +7,10 @@
 import pytest
 
 from PIL.PdfParser import (
+    MAX_SERIALIZE_NESTING,
     IndirectObjectDef,
     IndirectReference,
+    PdfArray,
     PdfBinary,
     PdfDict,
     PdfFormatError,
@@ -290,3 +292,56 @@ def test_long_trailer_chain(tmp_path: Path) -> None:
 
     with PdfParser(f=b) as pdf:
         pass
+
+
+def _deep_array(depth: int) -> PdfArray:
+    value = PdfArray([0])
+    for _ in range(depth - 1):
+        value = PdfArray([value])
+    return value
+
+
+def test_pdf_repr_max_nesting() -> None:
+    # Serializing a deeply nested value must not exhaust the call stack. This is
+    # the write-side sibling of the max_nesting guard used while parsing.
+    pdf_repr(_deep_array(MAX_SERIALIZE_NESTING))
+    with pytest.raises(PdfFormatError, match="maximum nesting depth exceeded"):
+        pdf_repr(_deep_array(MAX_SERIALIZE_NESTING + 1))
+    with pytest.raises(PdfFormatError, match="maximum nesting depth exceeded"):
+        bytes(_deep_array(2 * MAX_SERIALIZE_NESTING))
+
+
+def test_read_indirect_missing_object() -> None:
+    b = BytesIO()
+    with PdfParser(f=b, mode="wb") as pdf:
+        pdf.start_writing()
+        pdf.write_header()
+
+        pdf.write_catalog()
+        pdf.write_xref_and_trailer()
+
+    with PdfParser(f=b) as pdf:
+        # A reference to an object that is not in the xref table is malformed and
+        # must raise PdfFormatError rather than leaking a KeyError.
+        with pytest.raises(PdfFormatError, match="not found in xref table"):
+            pdf.read_indirect(IndirectReference(999, 0))
+
+
+def test_write_catalog_dangling_parent() -> None:
+    b = BytesIO()
+    with PdfParser(f=b, mode="wb") as pdf:
+        pdf.start_writing()
+        pdf.write_header()
+
+        page_id = pdf.next_object_id(0)
+        pdf.write_page(page_id, Parent=IndirectReference(999, 0))
+        pdf.pages = [page_id]
+        pdf.write_catalog()
+        pdf.write_xref_and_trailer()
+
+    with PdfParser(f=b) as pdf:
+        pdf.start_writing()
+        pdf.write_header()
+        # Rewriting a page whose /Parent is a dangling reference must not crash
+        pdf.write_catalog()
+        pdf.write_xref_and_trailer()

docs/releasenotes/13.1.0.rst

@@ -62,6 +62,20 @@ Blurring I;16 images
 :py:class:`~PIL.ImageFilter.GaussianBlur` and :py:class:`~PIL.ImageFilter.BoxBlur`
 now support I;16, I;16L, I;16B and I;16N images.
 
+Bounding recursion when writing PDFs
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Serializing a deeply nested value with ``PIL.PdfParser`` could exhaust the call
+stack and raise ``RecursionError``. Writing now stops at a fixed nesting depth
+and raises ``PdfFormatError`` instead.
+
+Rejecting dangling object references
+^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+Reading an indirect object that is missing from the xref table, or rewriting a
+PDF whose page tree is missing nodes, now raises ``PdfFormatError`` rather than
+a ``KeyError``.
+
 Minimum C version
 ^^^^^^^^^^^^^^^^^
 

src/PIL/PdfParser.py

@@ -264,7 +264,7 @@ def __bytes__(self) -> bytes:
 
 class PdfArray(list[Any]):
     def __bytes__(self) -> bytes:
-        return b"[ " + b" ".join(pdf_repr(x) for x in self) + b" ]"
+        return pdf_repr(self)
 
 
 class PdfDict(_DictBase):
@@ -302,17 +302,7 @@ def __getattr__(self, key: str) -> str | time.struct_time:
         return value
 
     def __bytes__(self) -> bytes:
-        out = bytearray(b"<<")
-        for key, value in self.items():
-            if value is None:
-                continue
-            value = pdf_repr(value)
-            out.extend(b"\n")
-            out.extend(bytes(PdfName(key)))
-            out.extend(b" ")
-            out.extend(value)
-        out.extend(b"\n>>")
-        return bytes(out)
+        return pdf_repr(self)
 
 
 class PdfBinary:
@@ -345,25 +335,35 @@ def decode(self, max_length: int = ImageFile.SAFEBLOCK) -> bytes:
             raise NotImplementedError(msg)
 
 
-def pdf_repr(x: Any) -> bytes:
+# Maximum object-nesting depth accepted when serializing PDF values. Serialization
+# recurses several Python frames per nesting level and runs at arbitrary call depth
+# (for example while an image is being saved), so this bound is kept conservative to
+# keep the stack far below Python's recursion limit and prevent stack exhaustion.
+MAX_SERIALIZE_NESTING = 100
+
+
+def pdf_repr(x: Any, _depth: int = 0) -> bytes:
+    if _depth > MAX_SERIALIZE_NESTING:
+        msg = "maximum nesting depth exceeded"
+        raise PdfFormatError(msg)
     if x is True:
         return b"true"
     elif x is False:
         return b"false"
     elif x is None:
         return b"null"
-    elif isinstance(x, (PdfName, PdfDict, PdfArray, PdfBinary)):
+    elif isinstance(x, (PdfName, PdfBinary)):
         return bytes(x)
+    elif isinstance(x, (PdfDict, dict)):
+        return _pdf_dict_repr(x, _depth)
+    elif isinstance(x, (PdfArray, list)):
+        return _pdf_array_repr(x, _depth)
     elif isinstance(x, (int, float)):
         return str(x).encode("us-ascii")
     elif isinstance(x, time.struct_time):
         return b"(D:" + time.strftime("%Y%m%d%H%M%SZ", x).encode("us-ascii") + b")"
-    elif isinstance(x, dict):
-        return bytes(PdfDict(x))
-    elif isinstance(x, list):
-        return bytes(PdfArray(x))
     elif isinstance(x, str):
-        return pdf_repr(encode_text(x))
+        return pdf_repr(encode_text(x), _depth)
     elif isinstance(x, bytes):
         # XXX escape more chars? handle binary garbage
         x = x.replace(b"\\", b"\\\\")
@@ -374,6 +374,24 @@ def pdf_repr(x: Any) -> bytes:
         return bytes(x)
 
 
+def _pdf_array_repr(x: Any, _depth: int) -> bytes:
+    return b"[ " + b" ".join(pdf_repr(item, _depth + 1) for item in x) + b" ]"
+
+
+def _pdf_dict_repr(x: Any, _depth: int) -> bytes:
+    out = bytearray(b"<<")
+    for key, value in x.items():
+        if value is None:
+            continue
+        value = pdf_repr(value, _depth + 1)
+        out.extend(b"\n")
+        out.extend(bytes(PdfName(key)))
+        out.extend(b" ")
+        out.extend(value)
+    out.extend(b"\n>>")
+    return bytes(out)
+
+
 class PdfParser:
     """Based on
     https://www.adobe.com/content/dam/acom/en/devnet/acrobat/pdfs/PDF32000_2008.pdf
@@ -494,7 +512,7 @@ def rewrite_pages(self) -> None:
         for i, page_ref in enumerate(self.orig_pages):
             page_info = self.cached_objects[page_ref]
             del self.xref_table[page_ref.object_id]
-            pages_tree_nodes_to_delete.append(page_info[PdfName(b"Parent")])
+            pages_tree_nodes_to_delete.append(page_info.get(PdfName(b"Parent")))
             if page_ref not in self.pages:
                 # the page has been deleted
                 continue
@@ -514,7 +532,9 @@ def rewrite_pages(self) -> None:
             processed_refs = []
             while pages_tree_node_ref and pages_tree_node_ref not in processed_refs:
                 processed_refs.append(pages_tree_node_ref)
-                pages_tree_node = self.cached_objects[pages_tree_node_ref]
+                pages_tree_node = self.cached_objects.get(pages_tree_node_ref)
+                if pages_tree_node is None:
+                    break
                 if pages_tree_node_ref.object_id in self.xref_table:
                     del self.xref_table[pages_tree_node_ref.object_id]
                 pages_tree_node_ref = pages_tree_node.get(b"Parent", None)
@@ -1074,6 +1094,10 @@ def read_xref_table(self, xref_section_offset: int) -> int:
         return offset
 
     def read_indirect(self, ref: IndirectReference, max_nesting: int = 900) -> Any:
+        check_format_condition(
+            ref[0] in self.xref_table,
+            f"object ID {ref[0]} not found in xref table",
+        )
         offset, generation = self.xref_table[ref[0]]
         check_format_condition(
             generation == ref[1],