classification
Title: Unexpected exceptions in plistlib.loads
Type: behavior Stage: resolved
Components: Library (Lib) Versions: Python 3.7, Python 3.6
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: serhiy.storchaka Nosy List: Ned Williamson, ronaldoussoren, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-10-30 04:04 by Ned Williamson, last changed 2017-11-20 18:08 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 4171 merged serhiy.storchaka, 2017-10-30 09:33
PR 4192 merged python-dev, 2017-10-31 12:07
Messages (11)
msg305205 - (view) Author: Ned Williamson (Ned Williamson) Date: 2017-10-30 04:04
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.
msg305206 - (view) Author: Ned Williamson (Ned Williamson) Date: 2017-10-30 04:06
The crashing version numbers are from testing on the release Python 3.5, but I think we can just fix this in 3.7+.
msg305207 - (view) Author: Ned Williamson (Ned Williamson) Date: 2017-10-30 04:16
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.
msg305208 - (view) Author: Ned Williamson (Ned Williamson) Date: 2017-10-30 04:19
```
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
```
msg305209 - (view) Author: Ned Williamson (Ned Williamson) Date: 2017-10-30 04:24
```
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
```
msg305213 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-30 07:02
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.
msg305216 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-30 10:10
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.
msg305254 - (view) Author: Ned Williamson (Ned Williamson) Date: 2017-10-30 21:44
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.
msg305292 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-31 12:05
New changeset db91e0fe2417f075693a194a492b1699829871e7 by Serhiy Storchaka in branch 'master':
bpo-31897: Convert unexpected errors when read bogus binary plists into InvalidFileException. (#4171)
https://github.com/python/cpython/commit/db91e0fe2417f075693a194a492b1699829871e7
msg305297 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-31 13:58
New changeset 6969d368c43d4c97e5f7b7b22904305ec68f79ba by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
bpo-31897: Convert unexpected errors when read bogus binary plists into InvalidFileException. (GH-4171) (#4192)
https://github.com/python/cpython/commit/6969d368c43d4c97e5f7b7b22904305ec68f79ba
msg306573 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-11-20 18:08
Opened issue32072 for infinite recursion and related issues.
History
Date User Action Args
2017-11-20 18:08:17serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg306573

stage: patch review -> resolved
2017-10-31 13:58:57serhiy.storchakasetmessages: + msg305297
2017-10-31 12:07:10python-devsetpull_requests: + pull_request4162
2017-10-31 12:05:55serhiy.storchakasetmessages: + msg305292
2017-10-30 21:44:49Ned Williamsonsetmessages: + msg305254
2017-10-30 10:10:13serhiy.storchakasetmessages: + msg305216
2017-10-30 09:34:39serhiy.storchakasetnosy: + ronaldoussoren
2017-10-30 09:33:59serhiy.storchakasetkeywords: + patch
stage: patch review
pull_requests: + pull_request4140
2017-10-30 07:03:45serhiy.storchakasettitle: RecursionError in plistlib.loads -> Unexpected exceptions in plistlib.loads
2017-10-30 07:02:48serhiy.storchakasetassignee: serhiy.storchaka
type: crash -> behavior
components: + Library (Lib)
versions: + Python 3.6
nosy: + serhiy.storchaka

messages: + msg305213
2017-10-30 04:24:28Ned Williamsonsetmessages: + msg305209
2017-10-30 04:19:11Ned Williamsonsetmessages: + msg305208
2017-10-30 04:16:44Ned Williamsonsetmessages: + msg305207
2017-10-30 04:06:06Ned Williamsonsetmessages: + msg305206
2017-10-30 04:04:14Ned Williamsoncreate