From b8e59f77e65ba4caeda8d910bd66df01a468cbea Mon Sep 17 00:00:00 2001 From: Ned Deily Date: Sat, 28 May 2011 02:19:19 -0700 Subject: Issue #985064: Make plistlib more resilient to faulty input plists. Patch by Mher Movsisyan. --- Lib/plistlib.py | 59 +++++++++++++++++++++++++++++------------------ Lib/test/test_plistlib.py | 26 +++++++++++++++++++++ Misc/ACKS | 1 + Misc/NEWS | 3 +++ 4 files changed, 67 insertions(+), 22 deletions(-) diff --git a/Lib/plistlib.py b/Lib/plistlib.py index fbba791..fe622ad 100644 --- a/Lib/plistlib.py +++ b/Lib/plistlib.py @@ -68,13 +68,15 @@ def readPlist(pathOrFile): usually is a dictionary). """ didOpen = False - if isinstance(pathOrFile, str): - pathOrFile = open(pathOrFile, 'rb') - didOpen = True - p = PlistParser() - rootObject = p.parse(pathOrFile) - if didOpen: - pathOrFile.close() + try: + if isinstance(pathOrFile, str): + pathOrFile = open(pathOrFile, 'rb') + didOpen = True + p = PlistParser() + rootObject = p.parse(pathOrFile) + finally: + if didOpen: + pathOrFile.close() return rootObject @@ -83,15 +85,17 @@ def writePlist(rootObject, pathOrFile): file name or a (writable) file object. """ didOpen = False - if isinstance(pathOrFile, str): - pathOrFile = open(pathOrFile, 'wb') - didOpen = True - writer = PlistWriter(pathOrFile) - writer.writeln("") - writer.writeValue(rootObject) - writer.writeln("") - if didOpen: - pathOrFile.close() + try: + if isinstance(pathOrFile, str): + pathOrFile = open(pathOrFile, 'wb') + didOpen = True + writer = PlistWriter(pathOrFile) + writer.writeln("") + writer.writeValue(rootObject) + writer.writeln("") + finally: + if didOpen: + pathOrFile.close() def readPlistFromBytes(data): @@ -352,7 +356,6 @@ class Data: def __repr__(self): return "%s(%s)" % (self.__class__.__name__, repr(self.data)) - class PlistParser: def __init__(self): @@ -362,11 +365,11 @@ class PlistParser: def parse(self, fileobj): from xml.parsers.expat import ParserCreate - parser = ParserCreate() - parser.StartElementHandler = self.handleBeginElement - parser.EndElementHandler = self.handleEndElement - parser.CharacterDataHandler = self.handleData - parser.ParseFile(fileobj) + self.parser = ParserCreate() + self.parser.StartElementHandler = self.handleBeginElement + self.parser.EndElementHandler = self.handleEndElement + self.parser.CharacterDataHandler = self.handleData + self.parser.ParseFile(fileobj) return self.root def handleBeginElement(self, element, attrs): @@ -385,12 +388,18 @@ class PlistParser: def addObject(self, value): if self.currentKey is not None: + if not isinstance(self.stack[-1], type({})): + raise ValueError("unexpected element at line %d" % + self.parser.CurrentLineNumber) self.stack[-1][self.currentKey] = value self.currentKey = None elif not self.stack: # this is the root object self.root = value else: + if not isinstance(self.stack[-1], type([])): + raise ValueError("unexpected element at line %d" % + self.parser.CurrentLineNumber) self.stack[-1].append(value) def getData(self): @@ -405,9 +414,15 @@ class PlistParser: self.addObject(d) self.stack.append(d) def end_dict(self): + if self.currentKey: + raise ValueError("missing value for key '%s' at line %d" % + (self.currentKey,self.parser.CurrentLineNumber)) self.stack.pop() def end_key(self): + if self.currentKey or not isinstance(self.stack[-1], type({})): + raise ValueError("unexpected key at line %d" % + self.parser.CurrentLineNumber) self.currentKey = self.getData() def begin_array(self, attrs): diff --git a/Lib/test/test_plistlib.py b/Lib/test/test_plistlib.py index b9a46b7..ccda920 100644 --- a/Lib/test/test_plistlib.py +++ b/Lib/test/test_plistlib.py @@ -175,6 +175,32 @@ class TestPlistlib(unittest.TestCase): self.assertEqual(test1, result1) self.assertEqual(test2, result2) + def test_invalidarray(self): + for i in ["key inside an array", + "key inside an array23", + "key inside an array3"]: + self.assertRaises(ValueError, plistlib.readPlistFromBytes, + ("%s"%i).encode()) + + def test_invaliddict(self): + for i in ["kcompound key", + "single key", + "missing key", + "k1v15.3" + "k1k2double key"]: + self.assertRaises(ValueError, plistlib.readPlistFromBytes, + ("%s"%i).encode()) + self.assertRaises(ValueError, plistlib.readPlistFromBytes, + ("%s"%i).encode()) + + def test_invalidinteger(self): + self.assertRaises(ValueError, plistlib.readPlistFromBytes, + b"not integer") + + def test_invalidreal(self): + self.assertRaises(ValueError, plistlib.readPlistFromBytes, + b"not real") + def test_main(): support.run_unittest(TestPlistlib) diff --git a/Misc/ACKS b/Misc/ACKS index 80fda10..c698d73 100644 --- a/Misc/ACKS +++ b/Misc/ACKS @@ -563,6 +563,7 @@ Skip Montanaro Paul Moore Derek Morr James A Morrison +Mher Movsisyan Sjoerd Mullender Sape Mullender Michael Muller diff --git a/Misc/NEWS b/Misc/NEWS index dada5c2..ec1bbf2 100644 --- a/Misc/NEWS +++ b/Misc/NEWS @@ -80,6 +80,9 @@ Core and Builtins Library ------- +- Issue #985064: Make plistlib more resilient to faulty input plists. + Patch by Mher Movsisyan. + - Issue #12175: RawIOBase.readall() now returns None if read() returns None. - Issue #12175: FileIO.readall() now raises a ValueError instead of an IOError -- cgit v0.12