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

Issues with binary plists #76253

Closed
serhiy-storchaka opened this issue Nov 18, 2017 · 10 comments
Closed

Issues with binary plists #76253

serhiy-storchaka opened this issue Nov 18, 2017 · 10 comments
Assignees
Labels
3.7 (EOL) end of life stdlib Python modules in the Lib dir type-security A security issue

Comments

@serhiy-storchaka
Copy link
Member

BPO 32072
Nosy @ronaldoussoren, @larryhastings, @ned-deily, @serhiy-storchaka
PRs
  • bpo-32072: Fix issues with binary plists. #4455
  • [3.6] bpo-32072: Fix issues with binary plists. (GH-4455) #4654
  • [3.5] bpo-32072: Fix issues with binary plists. (GH-4455) #4656
  • [3.4] bpo-32072: Fix issues with binary plists. (GH-4455) #4658
  • 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 2018-01-31.15:42:32.404>
    created_at = <Date 2017-11-18.19:06:16.453>
    labels = ['type-security', '3.7', 'library']
    title = 'Issues with binary plists'
    updated_at = <Date 2019-05-10.18:17:40.640>
    user = 'https://github.com/serhiy-storchaka'

    bugs.python.org fields:

    activity = <Date 2019-05-10.18:17:40.640>
    actor = 'ned.deily'
    assignee = 'serhiy.storchaka'
    closed = True
    closed_date = <Date 2018-01-31.15:42:32.404>
    closer = 'serhiy.storchaka'
    components = ['Library (Lib)']
    creation = <Date 2017-11-18.19:06:16.453>
    creator = 'serhiy.storchaka'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 32072
    keywords = ['patch']
    message_count = 10.0
    messages = ['306493', '307145', '307344', '307347', '309059', '310411', '310497', '311336', '311339', '311595']
    nosy_count = 4.0
    nosy_names = ['ronaldoussoren', 'larry', 'ned.deily', 'serhiy.storchaka']
    pr_nums = ['4455', '4654', '4656', '4658']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue32072'
    versions = ['Python 3.4', 'Python 3.5', 'Python 3.6', 'Python 3.7']

    @serhiy-storchaka
    Copy link
    Member Author

    plistlib creates a new objects when read references instead of using
    already read object.

    As result it doesn't preserve identity:

    >>> import plistlib
    >>> a = [['spam']]*2
    >>> a[0] is a[1]
    True
    >>> b = plistlib.loads(plistlib.dumps(a, fmt=plistlib.FMT_BINARY))
    >>> b == a
    True
    >>> b[0] is b[1]
    False

    And plistlib.loads() is vulnerable to plists containing cyclic
    references (as was exposed in bpo-31897). For example,
    plistlib.loads(b'bplist00\xa1\x00\x08\x00\x00\x00\x00\x00\x00\x01\x01\x00\x00\x00\x00\x00\x00\x00\x01\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x00\x0a')
    could return a list containing itself, but it is failed with
    RecursionError.

    plistlib.dumps() preserves reference in the output, but it saves
    redundant copies. For example plistlib.dumps([[]]*5,
    fmt=plistlib.FMT_BINARY) saves a list containing 5 identical empty
    lists, but it saves an empty list 5 times, and only the last copy is
    used. The other 4 copies are not referenced and just spent the file
    volume and the space of reference numbers. Saving
    [[[[['spam']*100]*100]*100]*100]*100 will result in a multigigabyte,
    while less than a kilobyte would be enough for saving it. Loading
    properly saved [[[[['spam']*100]*100]*100]*100]*100 withe the current
    plistlib.loads() will cause consuming many gigabytes of memory.

    1. The issues with plistlib.dumps() are:
      1a) Inefficient saving data with references. This is minor resource usage issue.
      1b) Impossibility to save a data with cyclic references. This is a
      lack of a feature.

    2. The issues with plistlib.loads() are:
      2a) Inefficient loading data with references. This can be not just a
      resource usage issue, but a security issue. Loading an malicious input
      data smaller than 100 byte ([[[...]*2]*2]*2) can cause consuming many
      gigabytes of memory.
      2b) Impossibility to load a data with cyclic references. This is a
      lack of a feature, but can be lesser security issue. Small malicious
      input can cause RecursionError. If the recursion limit is set high and
      you are unlucky it can cause a stack overflow.

    Security issues affect you only when you load plists from untrusted sources.

    Adding the proper support of references could be considered a new
    feature, but taking to account security issues it should be backported
    up to 3.4 when the support of binary plists was added.

    @serhiy-storchaka serhiy-storchaka added the 3.7 (EOL) end of life label Nov 18, 2017
    @serhiy-storchaka serhiy-storchaka self-assigned this Nov 18, 2017
    @serhiy-storchaka serhiy-storchaka added stdlib Python modules in the Lib dir type-security A security issue labels Nov 18, 2017
    @serhiy-storchaka
    Copy link
    Member Author

    Ronald, could you please make a review? I want to merge this before 3.7.0a3.

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset a897aee by Serhiy Storchaka in branch 'master':
    bpo-32072: Fix issues with binary plists. (bpo-4455)
    a897aee

    @serhiy-storchaka
    Copy link
    Member Author

    New changeset 8cd3108 by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
    bpo-32072: Fix issues with binary plists. (GH-4455) (bpo-4654)
    8cd3108

    @serhiy-storchaka
    Copy link
    Member Author

    For example:

    a = []
    for i in range(22):
        a = [a, a]
    
    b = plistlib.dumps(a, fmt=plistlib.FMT_BINARY)

    The result is 130 bytes long on patched plistlib. But plistlib.dumps(b) will expand to a structure consuming almost a gigabyte of memory on unpatched plistlib. Increasing the level of nesting by one will duplicate memory consumption, so it is easy to consume all available memory on any computer.

    @larryhastings
    Copy link
    Contributor

    New changeset c59731d by larryhastings (Serhiy Storchaka) in branch '3.4':
    [3.4] bpo-32072: Fix issues with binary plists. (GH-4455) (bpo-4658)
    c59731d

    @larryhastings
    Copy link
    Contributor

    New changeset 43f014d by larryhastings (Serhiy Storchaka) in branch '3.5':
    [3.5] bpo-32072: Fix issues with binary plists. (GH-4455) (bpo-4656)
    43f014d

    @serhiy-storchaka
    Copy link
    Member Author

    Thanks Larry.

    @larryhastings
    Copy link
    Contributor

    Thank you for the fix! I just wish I knew what plists were ;-)

    @ronaldoussoren
    Copy link
    Contributor

    @larry: plists are Apple's equivalent to Windows INI files ;-)

    @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
    Labels
    3.7 (EOL) end of life stdlib Python modules in the Lib dir type-security A security issue
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants