Skip to content

Avoid entity expansion attacks in Element Tree #68426

Description

@vadmium
BPO 24238
Nosy @tiran, @vadmium, @serhiy-storchaka
Files
  • etree_20130519.patch
  • etree-entities.v2.patch
  • 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 = None
    closed_at = None
    created_at = <Date 2015-05-19.11:17:59.200>
    labels = ['type-security', 'expert-XML']
    title = 'Avoid entity expansion attacks in Element Tree'
    updated_at = <Date 2016-06-04.06:48:58.706>
    user = 'https://github.com/vadmium'

    bugs.python.org fields:

    activity = <Date 2016-06-04.06:48:58.706>
    actor = 'martin.panter'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['XML']
    creation = <Date 2015-05-19.11:17:59.200>
    creator = 'martin.panter'
    dependencies = []
    files = ['39430', '43185']
    hgrepos = []
    issue_num = 24238
    keywords = ['patch']
    message_count = 2.0
    messages = ['243577', '267238']
    nosy_count = 3.0
    nosy_names = ['christian.heimes', 'martin.panter', 'serhiy.storchaka']
    pr_nums = []
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue24238'
    versions = ['Python 3.6']

    Activity

    1. vadmium commented on May 19, 2015

      @vadmium
      MemberAuthor

      This patch could be the basis of an alternative to Christian Heimes’s patch in bpo-17239. It adds a parser flag to the Element Tree modules so that they will immediately raise an exception when an entity declaration is encountered. I believe this should be sufficient to avoid DOS vulnerabilities like the Billion Laughs attack, where a small XML entity reference expands into a large string, and/or involves a large number of entity expansions.

      I think the advantage of this patch over the patch in bpo-17239 is this one should work on the current Expat library (which I understand Python can load externally). The other patch modifies the Expat library itself, so would only be useful when Python’s internal Expat library is being used (or the external Expat library was also patched in a similar manner).

      The disadvantage of this patch is that it disables handling XML data as soon as an entity is declared, even if the entities are not actually used, or they are only used in a non-malicious way. The other patch allows a limited amount of entity expansion.

      I would like some feedback on:

      • What others think of the basic approach, compared with Christian’s approach in bpo-17239
      • If reject_entities=True should be switched on by default, which could break compatibility, but could be sensible for most cases of basic XML parsing
      • If my changes to the examples in the documentation are excessive
      • If other Element Tree APIs should be modified similarly to XMLParser

      So far I have only changed the XMLParser class. The following APIs accept a parser object, so can also avoid the vulnerability by passing a custom parser object:

      • fromstringlist()
      • iterparse(), though “parser” is listed as deprecated (by bpo-17741)
      • parse() (module-level function)
      • XML()
      • XMLID()
      • ElementTree.parse() (method of ElementTree class)

      These APIs don’t have a custom parser object, so they are still always vulnerable:

      • fromstring()
      • XMLPullParser
    2. vadmium commented on Jun 4, 2016

      @vadmium
      MemberAuthor

      Today I discovered that Christian’s defusedxml project already does the same sort of thing. The difference is he calls the parameter forbid_entities. So I have updated my patch and changed the name from reject_entities to forbid_entities for compatibility.

    3. transferred this issue fromon Apr 10, 2022
    4. serhiy-storchaka commented on Sep 1, 2026

      @serhiy-storchaka
      Member

      The attacks which this issue is about are now prevented by Expat itself. Expat 2.4.0 added protection against input amplification, enabled by default, and CPython bundles 2.8.4. Both the "billion laughs" and the "quadratic blowup" documents are rejected with "limit on input amplification factor (from DTD and entities) breached". External entities are not fetched either -- they are reported as undefined.

      forbid_entities would now only add the ability to reject a document because it declares an entity, even an unused or harmless one. This is stricter than needed, and it was the disadvantage of the approach mentioned in the first message.

      What is still missing is the ability to tune the limits: the Python implementation exposes the underlying parser as XMLParser.parser, so SetBillionLaughsAttackProtectionMaximumAmplification() can be called on it, but the C implementation does not expose it. This is a part of gh-63682.

      Closing as obsolete.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions