Skip to content

XML vulnerabilities in Python #61441

Open
Open
XML vulnerabilities in Python#61441
Labels
extension-modulesC modules in the Modules dirstdlibStandard Library Python modules in the Lib/ directorytopic-XMLtype-securityA security issue

Description

@tiran
tiran
opened on Feb 19, 2013
BPO 17239
Nosy @warsaw, @birkenfeld, @rhettinger, @pitrou, @scoder, @larryhastings, @tiran, @benjaminp, @jwilk, @ned-deily, @mcepl, @ezio-melotti, @mitar, @vadmium, @serhiy-storchaka, @zooba
PRs
  • bpo-17239: Disable external entities in SAX parser #9217
  • gh-61441: XML entity expansion limitation #9265
  • [3.7] bpo-17239: Disable external entities in SAX parser (GH-9217) #9511
  • [3.6] bpo-17239: Disable external entities in SAX parser (GH-9217) #9512
    		•
    Dependencies
  • bpo-17318: xml.sax and xml.dom fetch DTDs by default (✅)
  • bpo-24238: Avoid entity expansion attacks in Element Tree
    • Files
  • xmlbomb_20130219.patch
  • xmlbomb_20150518.patch: Merged to 3.5
    		•

    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 2013-02-19.15:35:41.914>
labels = ['type-security', 'expert-XML', '3.8', '3.9', 'extension-modules', '3.7', 'library']
title = 'XML vulnerabilities in Python'
updated_at = <Date 2021-11-08.16:56:41.595>
user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2021-11-08.16:56:41.595>
actor = 'vstinner'
assignee = 'none'
closed = False
closed_date = None
closer = None
components = ['Extension Modules', 'Library (Lib)', 'XML']
creation = <Date 2013-02-19.15:35:41.914>
creator = 'christian.heimes'
dependencies = ['17318', '24238']
files = ['29122', '39415']
hgrepos = []
issue_num = 17239
keywords = ['patch']
message_count = 23.0
messages = ['182393', '184285', '184289', '184387', '185053', '243450', '243469', '243581', '324416', '324685', '325562', '325573', '325586', '325590', '325595', '325610', '325642', '325648', '325702', '325738', '326144', '326228', '326229']
nosy_count = 20.0
nosy_names = ['barry', 'georg.brandl', 'rhettinger', 'pitrou', 'scoder', 'larry', 'christian.heimes', 'benjamin.peterson', 'jwilk', 'ned.deily', 'mcepl', 'ezio.melotti', 'Arfrever', 'eli.bendersky', 'mitar', 'martin.panter', 'serhiy.storchaka', 'franck', 'steve.dower', 'rsandwick3']
pr_nums = ['9217', '9265', '9511', '9512']
priority = 'critical'
resolution = None
stage = 'patch review'
status = 'open'
superseder = None
type = 'security'
url = 'https://bugs.python.org/issue17239'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9']

    bpo-24238 is #68426 which remains open.

    Activity

    1. tiran commented on Feb 19, 2013

      @tiran
      tiran
      on Feb 19, 2013
      MemberAuthor

      Experimental fix for XML vulnerabilities against default. It's NOT ready and needs lots of polishing.

      https://pypi.python.org/pypi/defusedxml contains explanations of all issues
      https://pypi.python.org/pypi/defusedexpat is a standalone version of part of the patches for Python 2.6 to 3.3

    2. tiran
      added
      release-blocker
      extension-modulesC modules in the Modules dir
      stdlibStandard Library Python modules in the Lib/ directory
      topic-XML
      type-securityA security issue
      on Feb 19, 2013

    3. benjaminp commented on Mar 16, 2013

      @benjaminp
      benjaminp
      on Mar 16, 2013
      Contributor

      Since this has dragged on for quite a while, I'm probably just going to release 2.7.4 with a pointer to defusedxml in the release notes. (docs, though, perhaps)

    4. rhettinger commented on Mar 16, 2013

      @rhettinger
      rhettinger
      on Mar 16, 2013
      Contributor

      Since this has dragged on for quite a while, I'm probably
      just going to release 2.7.4 with a pointer to defusedxml
      in the release notes. (docs, though, perhaps)

      +1

    5. pitrou commented on Mar 17, 2013

      @pitrou
      pitrou
      on Mar 17, 2013
      Member

      Since this has dragged on for quite a while, I'm probably just going to
      release 2.7.4 with a pointer to defusedxml in the release notes. (docs,
      though, perhaps)

      +1 too.

    6. benjaminp commented on Mar 23, 2013

      @benjaminp
      benjaminp
      on Mar 23, 2013
      Contributor

      Not blocking 2.7.4 as discussed on mailing list.

    7. benjaminp
      removed
      release-blocker
      on Mar 23, 2013

    8. vadmium commented on May 18, 2015

      @vadmium
      vadmium
      on May 18, 2015
      Member

      I did a rough merge with current “default” (3.5 pre-release) branch so that I can have a closer look at this issue; see xmlbomb_20150518.patch for the result. There are some bits with Argument Clinit that need perfecting:

      • Unsure how to convert the ElementTree.XMLParser.__init__() signature (varied depending on XML_BOMB_PROTECTION compile-time flag) to Argument Clinic. So I just hard-coded it as if XML_BOMB_PROTECTION is always enabled. Why do we have to have a variable signature in the first place?

      • New pyexpat functions need porting to Argument Clinic.

    9. vadmium commented on May 18, 2015

      @vadmium
      vadmium
      on May 18, 2015
      Member

      I started looking at the lower Expat-level changes. Here are some thoughts, in the order that I thought them. :) But the end result is to investigate a different approach to disable entities in existing versions of Expat.

      Currently, it looks like max_entity_indirections = 0 is a special value meaning no limit. I think it would be better to use some other value such as None for this, and then 0 could disable all entity expansion (other than pre-defined entities like & &#xNNNN; etc).

      What is the benefit of having the indirection limit? I would have thought the entity expansion (character) limit on its own would already be effective at preventing nested expansion attacks like “billion laughs”. Even if the entity expanded to an empty string, all of the intermediate entity references are still included in the character count.

      I wonder if it would make more sense to have a total character limit instead, which would include the characters from custom entity expansions as already counted by the patch, but also count characters directly from the XML body. Why would you want to avoid 8 million characters from entity expansion, but allow 8 million characters of plain XML (or gzipped XML)? (I am not an XML expert, so I could be missing something obvious here.)

      Now I have discovered that it seems you can build Python to use an external Expat library, which won’t be affected by Christian’s fix (correct me if I am wrong). I think we should find a different solution that will also work with existing external Expat versions. Maybe setting EntityDeclHandler to raise an error would be good enough:

      >>> from xml.parsers import expat
>>> bomb = '<!DOCTYPE bomb [\n<!ENTITY a "" >\n<!ENTITY b "' + '&a;' * 1000 + '" >\n<!ENTITY c "' + '&b;' * 1000 + '" >\n]>\n<bomb a="' + '&c;' * 10 + '" />\n'
>>> p = expat.ParserCreate()
>>> p.Parse(bomb, True)  # Noticeable delay (DOS) while parsing
1
>>> p = expat.ParserCreate()
>>> def handler(*so_much_argh):
...     raise ValueError("Entity handling disabled")
... 
>>> p.EntityDeclHandler = handler
>>> p.Parse(bomb, True)  # Instant failure (no DOS)
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/build/python/src/Python-3.4.3/Modules/pyexpat.c", line 494, in EntityDecl
  File "<stdin>", line 2, in handler
ValueError: Entity handling disabled

      This solution has been suggested and implemented elsewhere:

    10. vadmium commented on May 19, 2015

      @vadmium
      vadmium
      on May 19, 2015
      Member

      I have opened bpo-24238 with a patch for Element Tree that uses my EntityDeclHandler technique, instead of patching Expat. I would be interested in other people’s thoughts on the approach.

    11. vstinner commented on Aug 31, 2018

      @vstinner
      vstinner
      on Aug 31, 2018
      Member

      This issue didn't get much attention in 5 years. The XML documentation starts with a big red warning:
      https://docs.python.org/dev/library/xml.html

      The warning is present in 2.7 and 3.4 as well:
      https://docs.python.org/2.7/library/xml.html
      https://docs.python.org/3.4/library/xml.html

      It seems like XML is getting less popular because of JSON becoming more popular (JSON obviously comes with its own set of security issues). It seems like less core developers care about XML.

      I suggest to:

      We just have to accept that core developers have limited availability and that documenting security issues is an acceptable tradeoff. I don't see any value of keeping these 3 issues open.

    12. 22 remaining items

    13. ahmedsayeed1982
      added
      3.8 (EOL)end of life
      and removed
      topic-XML
      3.9 (EOL)end of life
      on Nov 4, 2021
    14. eryksun
      added
      stdlibStandard Library Python modules in the Lib/ directory
      topic-XML
      3.7 (EOL)end of life
      3.9 (EOL)end of life
      on Nov 4, 2021
    15. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    16. picnixz
      added
      3.8 (EOL)end of life
      3.7 (EOL)end of life
      and removed
      3.8 (EOL)end of life
      3.7 (EOL)end of life
      3.9 (EOL)end of life
      on May 24, 2025
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      extension-modulesC modules in the Modules dirstdlibStandard Library Python modules in the Lib/ directorytopic-XMLtype-securityA security issue

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions