[3.8] bpo-42103: Improve validation of Plist files. (GH-22882) (GH-23116)

* Prevent some possible DoS attacks via providing invalid Plist files
  with extremely large number of objects or collection sizes.
* Raise InvalidFileException for too large bytes and string size instead of returning garbage.
* Raise InvalidFileException instead of ValueError for specific invalid datetime (NaN).
* Raise InvalidFileException instead of TypeError for non-hashable dict keys.
* Add more tests for invalid Plist files.

(cherry picked from commit 34637a0ce2)
This commit is contained in:
Serhiy Storchaka 2020-11-03 09:32:15 +02:00 committed by GitHub
parent 1e96de9ed4
commit 547d2bcc55
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
4 changed files with 366 additions and 61 deletions

View File

@ -600,7 +600,7 @@ class _BinaryPlistParser:
return self._read_object(top_object)
except (OSError, IndexError, struct.error, OverflowError,
UnicodeDecodeError):
ValueError):
raise InvalidFileException()
def _get_size(self, tokenL):
@ -616,7 +616,7 @@ class _BinaryPlistParser:
def _read_ints(self, n, size):
data = self._fp.read(size * n)
if size in _BINARY_FORMAT:
return struct.unpack('>' + _BINARY_FORMAT[size] * n, data)
return struct.unpack(f'>{n}{_BINARY_FORMAT[size]}', data)
else:
if not size or len(data) != size * n:
raise InvalidFileException()
@ -675,18 +675,25 @@ class _BinaryPlistParser:
elif tokenH == 0x40: # data
s = self._get_size(tokenL)
if self._use_builtin_types:
result = self._fp.read(s)
else:
result = Data(self._fp.read(s))
result = self._fp.read(s)
if len(result) != s:
raise InvalidFileException()
if not self._use_builtin_types:
result = Data(result)
elif tokenH == 0x50: # ascii string
s = self._get_size(tokenL)
result = self._fp.read(s).decode('ascii')
data = self._fp.read(s)
if len(data) != s:
raise InvalidFileException()
result = data.decode('ascii')
elif tokenH == 0x60: # unicode string
s = self._get_size(tokenL)
result = self._fp.read(s * 2).decode('utf-16be')
s = self._get_size(tokenL) * 2
data = self._fp.read(s)
if len(data) != s:
raise InvalidFileException()
result = data.decode('utf-16be')
elif tokenH == 0x80: # UID
# used by Key-Archiver plist files
@ -711,9 +718,11 @@ class _BinaryPlistParser:
obj_refs = self._read_refs(s)
result = self._dict_type()
self._objects[ref] = result
for k, o in zip(key_refs, obj_refs):
result[self._read_object(k)] = self._read_object(o)
try:
for k, o in zip(key_refs, obj_refs):
result[self._read_object(k)] = self._read_object(o)
except TypeError:
raise InvalidFileException()
else:
raise InvalidFileException()

View File

@ -2,6 +2,7 @@
import copy
import operator
import pickle
import struct
import unittest
import plistlib
import os
@ -118,6 +119,285 @@ XML_PLIST_WITH_ENTITY=b'''\
</plist>
'''
INVALID_BINARY_PLISTS = [
('too short data',
b''
),
('too large offset_table_offset and offset_size = 1',
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'\x00\x00\x00\x00\x00\x00\x00\x2a'
),
('too large offset_table_offset and nonstandard offset_size',
b'\x00\x00\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\x2c'
),
('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'
),
('too large top_object',
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\x01'
b'\x00\x00\x00\x00\x00\x00\x00\x09'
),
('integer overflow in top_object',
b'\x00\x08'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
b'\x00\x00\x00\x00\x00\x00\x00\x01'
b'\xff\xff\xff\xff\xff\xff\xff\xff'
b'\x00\x00\x00\x00\x00\x00\x00\x09'
),
('too large num_objects and offset_size = 1',
b'\x00\x08'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
b'\x00\x00\x00\x00\x00\x00\x00\xff'
b'\x00\x00\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00\x00\x09'
),
('too large num_objects and nonstandard offset_size',
b'\x00\x00\x00\x08'
b'\x00\x00\x00\x00\x00\x00\x03\x01'
b'\x00\x00\x00\x00\x00\x00\x00\xff'
b'\x00\x00\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00\x00\x09'
),
('extremally large num_objects (32 bit)',
b'\x00\x08'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
b'\x00\x00\x00\x00\x7f\xff\xff\xff'
b'\x00\x00\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00\x00\x09'
),
('extremally large num_objects (64 bit)',
b'\x00\x08'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
b'\x00\x00\x00\xff\xff\xff\xff\xff'
b'\x00\x00\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00\x00\x09'
),
('integer overflow in num_objects',
b'\x00\x08'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
b'\xff\xff\xff\xff\xff\xff\xff\xff'
b'\x00\x00\x00\x00\x00\x00\x00\x00'
b'\x00\x00\x00\x00\x00\x00\x00\x09'
),
('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'
),
('too large offset',
b'\x00\x2a'
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\x09'
),
('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'
),
('too large array size',
b'\xaf\x00\x01\xff\x00\x08\x0c'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
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\x0d'
),
('extremally large array size (32-bit)',
b'\xaf\x02\x7f\xff\xff\xff\x01\x00\x08\x0f'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
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\x10'
),
('extremally large array size (64-bit)',
b'\xaf\x03\x00\x00\x00\xff\xff\xff\xff\xff\x01\x00\x08\x13'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
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\x14'
),
('integer overflow in array size',
b'\xaf\x03\xff\xff\xff\xff\xff\xff\xff\xff\x01\x00\x08\x13'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
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\x14'
),
('too large reference index',
b'\xa1\x02\x00\x08\x0a'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
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 reference index',
b'\xa1\xff\xff\xff\xff\xff\xff\xff\xff\x00\x08\x11'
b'\x00\x00\x00\x00\x00\x00\x01\x08'
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\x12'
),
('too large bytes size',
b'\x4f\x00\x23\x41\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\x0c'
),
('extremally large bytes size (32-bit)',
b'\x4f\x02\x7f\xff\xff\xff\x41\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\x0f'
),
('extremally large bytes size (64-bit)',
b'\x4f\x03\x00\x00\x00\xff\xff\xff\xff\xff\x41\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\x13'
),
('integer overflow in bytes size',
b'\x4f\x03\xff\xff\xff\xff\xff\xff\xff\xff\x41\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\x13'
),
('too large ASCII size',
b'\x5f\x00\x23\x41\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\x0c'
),
('extremally large ASCII size (32-bit)',
b'\x5f\x02\x7f\xff\xff\xff\x41\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\x0f'
),
('extremally large ASCII size (64-bit)',
b'\x5f\x03\x00\x00\x00\xff\xff\xff\xff\xff\x41\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\x13'
),
('integer overflow in ASCII size',
b'\x5f\x03\xff\xff\xff\xff\xff\xff\xff\xff\x41\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\x13'
),
('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'
),
('too large UTF-16 size',
b'\x6f\x00\x13\x20\xac\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\x0e'
),
('extremally large UTF-16 size (32-bit)',
b'\x6f\x02\x4f\xff\xff\xff\x20\xac\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\x11'
),
('extremally large UTF-16 size (64-bit)',
b'\x6f\x03\x00\x00\x00\xff\xff\xff\xff\xff\x20\xac\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\x15'
),
('integer overflow in UTF-16 size',
b'\x6f\x03\xff\xff\xff\xff\xff\xff\xff\xff\x20\xac\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\x15'
),
('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'
),
('non-hashable key',
b'\xd1\x01\x01\xa0\x08\x0b'
b'\x00\x00\x00\x00\x00\x00\x01\x01'
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\x0c'
),
('too large datetime (datetime overflow)',
b'\x33\x42\x50\x00\x00\x00\x00\x00\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\x11'
),
('too large datetime (timedelta overflow)',
b'\x33\x42\xe0\x00\x00\x00\x00\x00\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\x11'
),
('invalid datetime (Infinity)',
b'\x33\x7f\xf0\x00\x00\x00\x00\x00\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\x11'
),
('invalid datetime (NaN)',
b'\x33\x7f\xf8\x00\x00\x00\x00\x00\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\x11'
),
]
class TestPlistlib(unittest.TestCase):
@ -559,6 +839,21 @@ class TestPlistlib(unittest.TestCase):
class TestBinaryPlistlib(unittest.TestCase):
@staticmethod
def decode(*objects, offset_size=1, ref_size=1):
data = [b'bplist00']
offset = 8
offsets = []
for x in objects:
offsets.append(offset.to_bytes(offset_size, 'big'))
data.append(x)
offset += len(x)
tail = struct.pack('>6xBBQQQ', offset_size, ref_size,
len(objects), 0, offset)
data.extend(offsets)
data.append(tail)
return plistlib.loads(b''.join(data), fmt=plistlib.FMT_BINARY)
def test_nonstandard_refs_size(self):
# Issue #21538: Refs and offsets are 24-bit integers
data = (b'bplist00'
@ -573,7 +868,7 @@ class TestBinaryPlistlib(unittest.TestCase):
def test_dump_duplicates(self):
# Test effectiveness of saving duplicated objects
for x in (None, False, True, 12345, 123.45, 'abcde', b'abcde',
for x in (None, False, True, 12345, 123.45, 'abcde', 'абвгд', b'abcde',
datetime.datetime(2004, 10, 26, 10, 33, 33),
plistlib.Data(b'abcde'), bytearray(b'abcde'),
[12, 345], (12, 345), {'12': 345}):
@ -612,6 +907,20 @@ class TestBinaryPlistlib(unittest.TestCase):
b = plistlib.loads(plistlib.dumps(a, fmt=plistlib.FMT_BINARY))
self.assertIs(b['x'], b)
def test_deep_nesting(self):
for N in [300, 100000]:
chunks = [b'\xa1' + (i + 1).to_bytes(4, 'big') for i in range(N)]
try:
result = self.decode(*chunks, b'\x54seed', offset_size=4, ref_size=4)
except RecursionError:
pass
else:
for i in range(N):
self.assertIsInstance(result, list)
self.assertEqual(len(result), 1)
result = result[0]
self.assertEqual(result, 'seed')
def test_large_timestamp(self):
# Issue #26709: 32-bit timestamp out of range
for ts in -2**31-1, 2**31:
@ -621,55 +930,37 @@ class TestBinaryPlistlib(unittest.TestCase):
data = plistlib.dumps(d, fmt=plistlib.FMT_BINARY)
self.assertEqual(plistlib.loads(data), d)
def test_load_singletons(self):
self.assertIs(self.decode(b'\x00'), None)
self.assertIs(self.decode(b'\x08'), False)
self.assertIs(self.decode(b'\x09'), True)
self.assertEqual(self.decode(b'\x0f'), b'')
def test_load_int(self):
self.assertEqual(self.decode(b'\x10\x00'), 0)
self.assertEqual(self.decode(b'\x10\xfe'), 0xfe)
self.assertEqual(self.decode(b'\x11\xfe\xdc'), 0xfedc)
self.assertEqual(self.decode(b'\x12\xfe\xdc\xba\x98'), 0xfedcba98)
self.assertEqual(self.decode(b'\x13\x01\x23\x45\x67\x89\xab\xcd\xef'),
0x0123456789abcdef)
self.assertEqual(self.decode(b'\x13\xfe\xdc\xba\x98\x76\x54\x32\x10'),
-0x123456789abcdf0)
def test_unsupported(self):
unsupported = [*range(1, 8), *range(10, 15),
0x20, 0x21, *range(0x24, 0x33), *range(0x34, 0x40)]
for i in [0x70, 0x90, 0xb0, 0xc0, 0xe0, 0xf0]:
unsupported.extend(i + j for j in range(16))
for token in unsupported:
with self.subTest(f'token {token:02x}'):
with self.assertRaises(plistlib.InvalidFileException):
self.decode(bytes([token]) + b'\x00'*16)
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)
for name, data in INVALID_BINARY_PLISTS:
with self.subTest(name):
with self.assertRaises(plistlib.InvalidFileException):
plistlib.loads(b'bplist00' + data, fmt=plistlib.FMT_BINARY)
class TestPlistlibDeprecated(unittest.TestCase):

View File

@ -0,0 +1,3 @@
:exc:`~plistlib.InvalidFileException` and :exc:`RecursionError` are now
the only errors caused by loading malformed binary Plist file (previously
ValueError and TypeError could be raised in some specific cases).

View File

@ -0,0 +1,2 @@
Prevented potential DoS attack via CPU and RAM exhaustion when processing
malformed Apple Property List files in binary format.