diff --git a/Lib/plistlib.py b/Lib/plistlib.py index 8262fb049e5869..2113a2dc57b4e1 100644 --- a/Lib/plistlib.py +++ b/Lib/plistlib.py @@ -557,7 +557,8 @@ def parse(self, fp): self._object_offsets = self._read_ints(num_objects, offset_size) return self._read_object(self._object_offsets[top_object]) - except (OSError, IndexError, struct.error): + except (OSError, IndexError, struct.error, OverflowError, + UnicodeDecodeError): raise InvalidFileException() def _get_size(self, tokenL): @@ -575,6 +576,8 @@ def _read_ints(self, n, size): if size in _BINARY_FORMAT: return struct.unpack('>' + _BINARY_FORMAT[size] * n, data) else: + if not size or len(data) != size * n: + raise InvalidFileException() return tuple(int.from_bytes(data[i: i + size], 'big') for i in range(0, size * n, size)) diff --git a/Lib/test/test_plistlib.py b/Lib/test/test_plistlib.py index 58b885f0228bdb..7dd179a2c71e9e 100644 --- a/Lib/test/test_plistlib.py +++ b/Lib/test/test_plistlib.py @@ -353,11 +353,13 @@ def test_controlcharacters(self): testString = "string containing %s" % c if i >= 32 or c in "\r\n\t": # \r, \n and \t are the only legal control chars in XML - plistlib.dumps(testString, fmt=plistlib.FMT_XML) + data = plistlib.dumps(testString, fmt=plistlib.FMT_XML) + if c != "\r": + self.assertEqual(plistlib.loads(data), testString) else: - self.assertRaises(ValueError, - plistlib.dumps, - testString) + with self.assertRaises(ValueError): + plistlib.dumps(testString, fmt=plistlib.FMT_XML) + plistlib.dumps(testString, fmt=plistlib.FMT_BINARY) def test_non_bmp_characters(self): pl = {'python': '\U0001f40d'} @@ -366,6 +368,14 @@ def test_non_bmp_characters(self): data = plistlib.dumps(pl, fmt=fmt) self.assertEqual(plistlib.loads(data), pl) + def test_lone_surrogates(self): + for fmt in ALL_FORMATS: + with self.subTest(fmt=fmt): + with self.assertRaises(UnicodeEncodeError): + plistlib.dumps('\ud8ff', fmt=fmt) + with self.assertRaises(UnicodeEncodeError): + plistlib.dumps('\udcff', fmt=fmt) + def test_nondictroot(self): for fmt in ALL_FORMATS: with self.subTest(fmt=fmt): @@ -442,6 +452,56 @@ def test_large_timestamp(self): data = plistlib.dumps(d, fmt=plistlib.FMT_BINARY) self.assertEqual(plistlib.loads(data), d) + def test_invalid_binary(self): + for data in [ + # too short data + b'', + # too large offset_table_offset and nonstandard offset_size + b'\x00\x08' + b'\x00\x00\x00\x00\x00\x00\x03\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x2a', + # integer overflow in offset_table_offset + b'\x00\x08' + b'\x00\x00\x00\x00\x00\x00\x01\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x00' + b'\xff\xff\xff\xff\xff\xff\xff\xff', + # offset_size = 0 + b'\x00\x08' + b'\x00\x00\x00\x00\x00\x00\x00\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x09', + # ref_size = 0 + b'\xa1\x01\x00\x08\x0a' + b'\x00\x00\x00\x00\x00\x00\x01\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x02' + b'\x00\x00\x00\x00\x00\x00\x00\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x0b', + # integer overflow in offset + b'\x00\xff\xff\xff\xff\xff\xff\xff\xff' + b'\x00\x00\x00\x00\x00\x00\x08\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x09', + # invalid ASCII + b'\x51\xff\x08' + b'\x00\x00\x00\x00\x00\x00\x01\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x0a', + # invalid UTF-16 + b'\x61\xd8\x00\x08' + b'\x00\x00\x00\x00\x00\x00\x01\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x01' + b'\x00\x00\x00\x00\x00\x00\x00\x00' + b'\x00\x00\x00\x00\x00\x00\x00\x0b', + ]: + with self.assertRaises(plistlib.InvalidFileException): + plistlib.loads(b'bplist00' + data, fmt=plistlib.FMT_BINARY) + class TestPlistlibDeprecated(unittest.TestCase): def test_io_deprecated(self): diff --git a/Misc/NEWS.d/next/Library/2017-10-30-11-04-56.bpo-31897.yjwdEb.rst b/Misc/NEWS.d/next/Library/2017-10-30-11-04-56.bpo-31897.yjwdEb.rst new file mode 100644 index 00000000000000..229472cb4898da --- /dev/null +++ b/Misc/NEWS.d/next/Library/2017-10-30-11-04-56.bpo-31897.yjwdEb.rst @@ -0,0 +1,2 @@ +plistlib now catches more errors when read binary plists and raises +InvalidFileException instead of unexpected exceptions.