Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unexpected exceptions in plistlib.loads #76078

Closed
NedWilliamson mannequin opened this issue Oct 30, 2017 · 11 comments
Closed

Unexpected exceptions in plistlib.loads #76078

NedWilliamson mannequin opened this issue Oct 30, 2017 · 11 comments
Assignees
@serhiy-storchaka
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@NedWilliamson
Copy link
Mannequin

NedWilliamson mannequin commented Oct 30, 2017

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
    Copy link
    Mannequin Author

    NedWilliamson mannequin commented Oct 30, 2017

    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 NedWilliamson mannequin added type-crash A hard crash of the interpreter, possibly with a core dump 3.7 (EOL) end of life labels Oct 30, 2017
    @NedWilliamson
    Copy link
    Mannequin Author

    NedWilliamson mannequin commented Oct 30, 2017

    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
    Copy link
    Mannequin Author

    NedWilliamson mannequin commented Oct 30, 2017

    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
    Copy link
    Mannequin Author

    NedWilliamson mannequin commented Oct 30, 2017 

    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
    Copy link
    Mannequin Author

    NedWilliamson mannequin commented Oct 30, 2017 

    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
    Copy link
    Member

    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 serhiy-storchaka added the stdlib Python modules in the Lib dir label Oct 30, 2017
    @serhiy-storchaka serhiy-storchaka self-assigned this Oct 30, 2017
    @serhiy-storchaka serhiy-storchaka added type-bug An unexpected behavior, bug, or error and removed type-crash A hard crash of the interpreter, possibly with a core dump labels Oct 30, 2017
    @serhiy-storchaka serhiy-storchaka changed the title RecursionError in plistlib.loads Unexpected exceptions in plistlib.loads Oct 30, 2017
    @serhiy-storchaka
    Copy link
    Member

    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
    Copy link
    Mannequin Author

    NedWilliamson mannequin commented Oct 30, 2017

    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.

    @serhiy-storchaka
    Copy link
    Member

    New changeset db91e0f by Serhiy Storchaka in branch 'master':
    bpo-31897: Convert unexpected errors when read bogus binary plists into InvalidFileException. (bpo-4171)
    db91e0f

    @serhiy-storchaka
    Copy link
    Member

    New changeset 6969d36 by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
    bpo-31897: Convert unexpected errors when read bogus binary plists into InvalidFileException. (GH-4171) (bpo-4192)
    6969d36

    @serhiy-storchaka
    Copy link
    Member

    Opened bpo-32072 for infinite recursion and related issues.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Assignees

    @serhiy-storchaka serhiy-storchaka

    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Milestone
    No milestone
    Development

    No branches or pull requests
    1 participant
    @serhiy-storchaka