Skip to content

Commit 6a8db47

Browse files
committed
gh-155397: Raise InvalidFileException for malformed XML plists in plistlib
_PlistParser.parse() called expat's ParseFile() with no exception translation, so two classes of malformed XML plist escaped as the underlying exception instead of the documented InvalidFileException: * XML that is not well-formed raised xml.parsers.expat.ExpatError. * An <?xml ...?> declaration naming an encoding unknown to Python's codec registry raised LookupError (this is what CIFuzz found in gh-152211). Neither exception type is a ValueError, so code written against the documented contract (catching InvalidFileException, or even just ValueError) did not catch them.
1 parent 998b890 commit 6a8db47

3 files changed

Lines changed: 32 additions & 2 deletions

File tree

Lib/plistlib.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -67,7 +67,7 @@
6767
import os
6868
import re
6969
import struct
70-
from xml.parsers.expat import ParserCreate
70+
from xml.parsers.expat import ExpatError, ParserCreate
7171

7272

7373
PlistFormat = enum.Enum('PlistFormat', 'FMT_XML FMT_BINARY', module=__name__)
@@ -185,7 +185,15 @@ def parse(self, fileobj):
185185
self.parser.EndElementHandler = self.handle_end_element
186186
self.parser.CharacterDataHandler = self.handle_data
187187
self.parser.EntityDeclHandler = self.handle_entity_decl
188-
self.parser.ParseFile(fileobj)
188+
try:
189+
self.parser.ParseFile(fileobj)
190+
except (ExpatError, LookupError):
191+
# gh-155397: ExpatError is raised for XML that is not
192+
# well-formed, and LookupError for a <?xml ... ?> declaration
193+
# naming an unknown encoding; neither is a ValueError, so it
194+
# would otherwise escape uncaught instead of the documented
195+
# InvalidFileException.
196+
raise InvalidFileException()
189197
return self.root
190198

191199
def handle_entity_decl(self, entity_name, is_parameter_entity, value, base, system_id, public_id, notation_name):

Lib/test/test_plistlib.py

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -933,6 +933,24 @@ def test_xml_plist_with_entity_decl(self):
933933
"XML entity declarations are not supported"):
934934
plistlib.loads(XML_PLIST_WITH_ENTITY, fmt=plistlib.FMT_XML)
935935

936+
def test_xml_plist_not_well_formed(self):
937+
# gh-155397: malformed XML must raise InvalidFileException, not the
938+
# underlying xml.parsers.expat.ExpatError.
939+
with self.assertRaises(plistlib.InvalidFileException):
940+
plistlib.loads(b"<plist><dict>")
941+
with self.assertRaises(plistlib.InvalidFileException):
942+
plistlib.loads(b"<plist><foo></bar></plist>")
943+
with self.assertRaises(plistlib.InvalidFileException):
944+
plistlib.loads(b"<plist>&undefined_entity;</plist>")
945+
946+
def test_xml_plist_unknown_encoding(self):
947+
# gh-155397: an <?xml ... ?> declaration naming an encoding unknown
948+
# to Python must raise InvalidFileException, not the underlying
949+
# LookupError.
950+
with self.assertRaises(plistlib.InvalidFileException):
951+
plistlib.loads(
952+
b'<?xml version="1.0" encoding="BogusEncoding"?><plist></plist>')
953+
936954
def test_load_aware_datetime(self):
937955
dt = plistlib.loads(b"<plist><date>2023-12-10T08:03:30Z</date></plist>",
938956
aware_datetime=True)
Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
Fix :mod:`plistlib` to raise :exc:`~plistlib.InvalidFileException` instead
2+
of leaking the underlying :exc:`xml.parsers.expat.ExpatError` (for
3+
not-well-formed XML) or :exc:`LookupError` (for an ``<?xml ... ?>``
4+
declaration naming an unknown encoding) when parsing a malformed XML plist.

0 commit comments

Comments
 (0)