Unexpected exceptions in plistlib.loads

[NedWilliamson]

BPO	31897
Nosy	@ronaldoussoren, @serhiy-storchaka
PRs	bpo-31897: Convert unexpected errors when read bogus binary plists … #4171 [3.6] bpo-31897: Convert unexpected errors when read bogus binary plists into InvalidFileException. (GH-4171) #4192

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = 'https://github.com/serhiy-storchaka'
closed_at = <Date 2017-11-20.18:08:17.377>
created_at = <Date 2017-10-30.04:04:14.176>
labels = ['3.7', 'type-bug', 'library']
title = 'Unexpected exceptions in plistlib.loads'
updated_at = <Date 2017-11-20.18:08:17.375>
user = 'https://bugs.python.org/NedWilliamson'

bugs.python.org fields:

activity = <Date 2017-11-20.18:08:17.375>
actor = 'serhiy.storchaka'
assignee = 'serhiy.storchaka'
closed = True
closed_date = <Date 2017-11-20.18:08:17.377>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2017-10-30.04:04:14.176>
creator = 'Ned Williamson'
dependencies = []
files = []
hgrepos = []
issue_num = 31897
keywords = ['patch']
message_count = 11.0
messages = ['305205', '305206', '305207', '305208', '305209', '305213', '305216', '305254', '305292', '305297', '306573']
nosy_count = 3.0
nosy_names = ['ronaldoussoren', 'serhiy.storchaka', 'Ned Williamson']
pr_nums = ['4171', '4192']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue31897'
versions = ['Python 3.6', 'Python 3.7']

[NedWilliamson]

Hi,

The following program crashes for me using the current Python3.7 master:

import plistlib
plistlib.loads(b'\xdd\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00'
               b'\xda\x0cw\xb7\x00\x00\x00\x00\x00\x00\x00\xc7\x00'
               b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\xd6\xd5\x00'
               b'\x00\x00\x00\x00\x00\x00\x04\x00\x00\x00\x00\x00'
               b'\x00\x00\x00\x00\x10\x00\x00\x00\x00\x00\x00',
               fmt=plistlib.FMT_BINARY)

The last few lines look like

  File "/usr/lib/python3.5/plistlib.py", line 728, in _read_object
    ] = self._read_object(self._object_offsets[o])
  File "/usr/lib/python3.5/plistlib.py", line 728, in _read_object
    ] = self._read_object(self._object_offsets[o])
  File "/usr/lib/python3.5/plistlib.py", line 723, in _read_object
    key_refs = self._read_refs(s)
  File "/usr/lib/python3.5/plistlib.py", line 647, in _read_refs
    return self._read_ints(n, self._ref_size)
  File "/usr/lib/python3.5/plistlib.py", line 644, in _read_ints
    for i in range(0, size * n, size))
RecursionError: maximum recursion depth exceeded in comparison

This bug was found using an alpha version of python-fuzz.

[NedWilliamson]

The crashing version numbers are from testing on the release Python 3.5, but I think we can just fix this in 3.7+.

[NedWilliamson]

I'm filing related bugs under this same issue.

import plistlib
dat = b'Q\xe4\xfeAIAAAAAAAAwAAA\xc9A\xc1AAA\xc1AAAAAAA\x9cAAAAAAAAAAAAAAnAAA\x00\x00\x00\x00\x00\x00\x00\x0b\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00AA'
plistlib.loads(dat, fmt=plistlib.FMT_BINARY)

raises

Traceback (most recent call last):
  File "repro.py", line 3, in <module>
    plistlib.loads(dat, fmt=plistlib.FMT_BINARY)
  File "/usr/lib/python3.5/plistlib.py", line 1006, in loads
    fp, fmt=fmt, use_builtin_types=use_builtin_types, dict_type=dict_type)
  File "/usr/lib/python3.5/plistlib.py", line 997, in load
    return p.parse(fp)
  File "/usr/lib/python3.5/plistlib.py", line 623, in parse
    return self._read_object(self._object_offsets[top_object])
  File "/usr/lib/python3.5/plistlib.py", line 699, in _read_object
    result =  self._fp.read(s).decode('ascii')
UnicodeDecodeError: 'ascii' codec can't decode byte 0xe4 in position 0: ordinal not in range(128)

It seems only InvalidFileException should be raised by this function.

[NedWilliamson]

import plistlib
dat = b'AAAAAAAAAAAwAAA\xc9AAAAAAAAAAAAA\x9cAAAAAAAAAAAAAAAAAA\x00\x00\x00\x00\x00\x00\x00\x0b\x00\x00AAAAA\x04\xb2\xaaAAAAAA'
plistlib.loads(dat, fmt=plistlib.FMT_BINARY)

raises

Traceback (most recent call last):
  File "repro.py", line 3, in <module>
    plistlib.loads(dat, fmt=plistlib.FMT_BINARY)
  File "/usr/lib/python3.5/plistlib.py", line 1006, in loads
    fp, fmt=fmt, use_builtin_types=use_builtin_types, dict_type=dict_type)
  File "/usr/lib/python3.5/plistlib.py", line 997, in load
    return p.parse(fp)
  File "/usr/lib/python3.5/plistlib.py", line 621, in parse
    self._fp.seek(offset_table_offset)
OverflowError: Python int too large to convert to C ssize_t

[NedWilliamson]

import plistlib
dat = b'\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00AAAnAAA'
plistlib.loads(dat, fmt=plistlib.FMT_BINARY)

raises

Traceback (most recent call last):
  File "repro.py", line 3, in <module>
    plistlib.loads(dat, fmt=plistlib.FMT_BINARY)
  File "/usr/lib/python3.5/plistlib.py", line 1006, in loads
    fp, fmt=fmt, use_builtin_types=use_builtin_types, dict_type=dict_type)
  File "/usr/lib/python3.5/plistlib.py", line 997, in load
    return p.parse(fp)
  File "/usr/lib/python3.5/plistlib.py", line 622, in parse
    self._object_offsets = self._read_ints(num_objects, offset_size)
  File "/usr/lib/python3.5/plistlib.py", line 644, in _read_ints
    for i in range(0, size * n, size))
ValueError: range() arg 3 must not be zero

[serhiy-storchaka]

Thank you for your report Ned. But there are no crashes. The term crash means a segmentation fault or similar error that causes the interpreter to exit immediately.

[serhiy-storchaka]

PR 4171 fixes the following errors:

1. OverflowError is raised by seek() for too large offsets of objects or the offset table.

2. Since read() past the file returns b'' and int.from_bytes() used for non-standard sizes accepts b'', bogus offsets and references can be read. This can cause an infinity recursion.

3. The zero size of offsets or references causes ValueError. This is implementation detail.

4. Unicode errors of decoding from invalid ASCII and UTF-8.

It doesn't verify the binary plist, a bogus plist can be successfully parsed to a bogus data. And it doesn't prevent infinity recursion when read cyclic references.

[NedWilliamson]

Thank you for the quick PR! I will report as behavior next time. I'm also following the library reference and reporting only unexpected exceptions.

I trust you to reject any bugs that are expected functionality.

I may follow up with additional testcases once the first PR is accepted.