diff --git a/Tests/test_pdfparser.py b/Tests/test_pdfparser.py index b8e9a434da7..2f4edc055be 100644 --- a/Tests/test_pdfparser.py +++ b/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() diff --git a/docs/releasenotes/13.1.0.rst b/docs/releasenotes/13.1.0.rst index 380154ae109..67c5b911f17 100644 --- a/docs/releasenotes/13.1.0.rst +++ b/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 ^^^^^^^^^^^^^^^^^ diff --git a/src/PIL/PdfParser.py b/src/PIL/PdfParser.py index 70bf71b2b1b..67070542d96 100644 --- a/src/PIL/PdfParser.py +++ b/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],