Skip to content

CVE-2012-0876 (hash table collisions CPU usage DoS) for embedded copy of expat #58442

Closed
Closed
CVE-2012-0876 (hash table collisions CPU usage DoS) for embedded copy of expat#58442
Assignees
gpshead
Labels
release-blockertopic-XML

Description

@davidmalcolm
davidmalcolm
opened on Mar 9, 2012
BPO 14234
Nosy @warsaw, @birkenfeld, @gpshead, @amauryfa, @pitrou, @benjaminp, @davidmalcolm, @JimJJewett
Files
  • expat-hash-randomization.patch: Patch against devel branch to add fix for CVE-2012-0876 to embedded copy of expat and to use it in pyexpat
  • expat-hash-randomization-002.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 = 'https://github.com/gpshead'
closed_at = <Date 2012-03-15.01:28:27.440>
created_at = <Date 2012-03-09.00:56:30.606>
labels = ['expert-XML', 'release-blocker']
title = 'CVE-2012-0876 (hash table collisions CPU usage DoS) for embedded copy of expat'
updated_at = <Date 2012-03-18.19:40:10.000>
user = 'https://github.com/davidmalcolm'

    bugs.python.org fields:

    activity = <Date 2012-03-18.19:40:10.000>
actor = 'python-dev'
assignee = 'gregory.p.smith'
closed = True
closed_date = <Date 2012-03-15.01:28:27.440>
closer = 'gregory.p.smith'
components = ['XML']
creation = <Date 2012-03-09.00:56:30.606>
creator = 'dmalcolm'
dependencies = []
files = ['24762', '24831']
hgrepos = []
issue_num = 14234
keywords = ['patch']
message_count = 27.0
messages = ['155198', '155217', '155258', '155262', '155263', '155474', '155516', '155687', '155690', '155694', '155695', '155720', '155722', '155785', '155791', '155808', '155811', '155812', '155827', '155829', '155832', '155835', '155840', '155921', '155956', '156086', '156267']
nosy_count = 10.0
nosy_names = ['barry', 'georg.brandl', 'gregory.p.smith', 'amaury.forgeotdarc', 'pitrou', 'benjamin.peterson', 'Arfrever', 'dmalcolm', 'python-dev', 'Jim.Jewett']
pr_nums = []
priority = 'release blocker'
resolution = 'fixed'
stage = None
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue14234'
versions = ['Python 2.6', 'Python 3.1', 'Python 2.7', 'Python 3.2', 'Python 3.3']

    Activity

    1. davidmalcolm commented on Mar 9, 2012

      @davidmalcolm
      davidmalcolm
      on Mar 9, 2012
      MemberAuthor

      Expat 2.1.0 Beta was recently announced:
      http://mail.libexpat.org/pipermail/expat-discuss/2012-March/002768.html
      which contains (among other things) a fix for a hash-collision denial-of-service attack (CVE-2012-0876)

      I'm attaching a patch which minimally backports the hash-collision fix part of expat 2.1.0 to the embedded copy of expat in the CPython source tree, and which adds a call to XML_SetHashSalt() to pyexpat when creating parsers. It reuses part of the hash secret from Py_HashSecret.

    2. davidmalcolm
      added
      topic-XML
      on Mar 9, 2012
    3. birkenfeld
      added
      release-blocker
      on Mar 9, 2012

    4. birkenfeld commented on Mar 9, 2012

      @birkenfeld
      birkenfeld
      on Mar 9, 2012
      Member

      I hope this can be integrated during the PyCon sprints?

    5. benjaminp commented on Mar 9, 2012

      @benjaminp
      benjaminp
      on Mar 9, 2012
      Contributor

      Since this has been approved upstream and the Python change is minimal, I think this can just be applied.

    6. pitrou commented on Mar 9, 2012

      @pitrou
      pitrou
      on Mar 9, 2012
      Member

      Note sure I understand: XML_SetHashSalt() takes a parser argument, but the hash secret is global?

    7. amauryfa commented on Mar 9, 2012

      @amauryfa
      amauryfa
      on Mar 9, 2012
      Contributor

      No, the salt is stored on the parser. See the line:

      +#define hash_secret_salt (parser->m_hash_secret_salt)

      Yes, expat code is confusing.

    8. gpshead commented on Mar 12, 2012

      @gpshead
      gpshead
      on Mar 12, 2012
      Member

      reviewing now.

    9. gpshead commented on Mar 12, 2012

      @gpshead
      gpshead
      on Mar 12, 2012
      Member

      Oddly, test_sax fails once this patch is applied (using 3.1). debugging now.

      test_sax
test test_sax failed -- Traceback (most recent call last):
  File "/home/greg/sandbox/python/cpython/3.1/Lib/xml/sax/expatreader.py", line 207, in feed
    self._parser.Parse(data, isFinal)
xml.parsers.expat.ExpatError: unbound prefix: line 1, column 59

During handling of the above exception, another exception occurred:

Traceback (most recent call last):
  File "/home/greg/sandbox/python/cpython/3.1/Lib/test/test_sax.py", line 310, in test_5027_1
    parser.parse(test_xml)
  File "/home/greg/sandbox/python/cpython/3.1/Lib/xml/sax/expatreader.py", line 107, in parse
    xmlreader.IncrementalParser.parse(self, source)
  File "/home/greg/sandbox/python/cpython/3.1/Lib/xml/sax/xmlreader.py", line 123, in parse
    self.feed(buffer)
  File "/home/greg/sandbox/python/cpython/3.1/Lib/xml/sax/expatreader.py", line 211, in feed
    self._err_handler.fatalError(exc)
  File "/home/greg/sandbox/python/cpython/3.1/Lib/xml/sax/handler.py", line 38, in fatalError
    raise exception
xml.sax._exceptions.SAXParseException: <unknown>:1:59: unbound prefix

    10. Arfrever commented on Mar 13, 2012

      @Arfrever
      Arfrevermannequin
      on Mar 13, 2012
      Mannequin

      Maybe it's related to: https://sourceforge.net/tracker/?func=detail&aid=3500861&group_id=10127&atid=110127

      (But I think that --with-system-expat should be recommended.)

    11. gpshead commented on Mar 13, 2012

      @gpshead
      gpshead
      on Mar 13, 2012
      Member

      sweet, thanks for the reference. that really looks like the problem.

    12. gpshead commented on Mar 14, 2012

      @gpshead
      gpshead
      on Mar 14, 2012
      Member

      uploaded an updated patch (against 3.1) with the changes from r1.168 to r1.170 xmlparse.c from the expat project. it fixes the test_sax issue.

      there is one other thing that needs fixing (next patch update).

      The test for the hash seed being == 0 that falls back to using the expat provided trivial time() based seed undesirable. We want a hash seed of 0 to be "disabled" matching the old behavior. this might require adding a flag indicating if the hash seed has been initialized or not.

      I'm also going to look at the possibility of using the Python interpreter's prefix and suffix values in some way rather than just prefix to avoid a potential of exposing the seed.

    13. gpshead commented on Mar 14, 2012

      @gpshead
      gpshead
      on Mar 14, 2012
      Member

      A test case for this is also needed.

      one that sets the hash seed via the environment variable to a different value for two subprocesses that parse and re-emit an xml document to confirm that all of the xml attributes are present but emitted in a different order indicating that attribute hash randomization was in effect is needed.

    14. gpshead
      self-assigned this
      on Mar 14, 2012

    15. gpshead commented on Mar 14, 2012

      @gpshead
      gpshead
      on Mar 14, 2012
      Member

      The existing pyexpat API doesn't give me a way to test if hash randomization is actually working so I'm going ahead without a specific test case for this.

      Attributes are either reported to xmlparser.SameElementHandler in a dictionary (unordered) or are reported in a list in the order they appeared on the element depending on the xmlparser.ordered_attributes bool.

    16. 6 remaining items

    17. gpshead
      assigned
      warsaw
      and unassigned
      gpshead
      on Mar 14, 2012

    18. python-dev commented on Mar 15, 2012

      @python-dev
      python-devmannequin
      on Mar 15, 2012
      Mannequin

      New changeset 9c8d066013ea by Barry Warsaw in branch '2.6':

    19. gpshead commented on Mar 15, 2012

      @gpshead
      gpshead
      on Mar 15, 2012
      Member

      One issue has been identified when compiling with --system-expat. if the system expat library does not have the hash salt support, compilation breaks.

      fixing now.

    20. gpshead commented on Mar 15, 2012

      @gpshead
      gpshead
      on Mar 15, 2012
      Member

      configure --with-system-expat was introduced in 2.7 and 3.2 so 2.6 and 3.1 are good to go for release candidates.

      patch tests are running now.

    21. python-dev commented on Mar 15, 2012

      @python-dev
      python-devmannequin
      on Mar 15, 2012
      Mannequin

      New changeset b2d4a6a9463e by Gregory P. Smith in branch '3.2':
      Fixes bpo-14234: fix for the previous commit, keep compilation when
      http://hg.python.org/cpython/rev/b2d4a6a9463e

      New changeset db27b7353400 by Gregory P. Smith in branch 'default':
      Fixes bpo-14234: fix for the previous commit, keep compilation when
      http://hg.python.org/cpython/rev/db27b7353400

      New changeset cb72aa8a8008 by Gregory P. Smith in branch '2.7':
      Fixes bpo-14234: fix for the previous commit, keep compilation when
      http://hg.python.org/cpython/rev/cb72aa8a8008

    22. gpshead commented on Mar 15, 2012

      @gpshead
      gpshead
      on Mar 15, 2012
      Member

      okay. it is time to cut the rc2 release candidates with these changes.

    23. gpshead
      closed this as completedon Mar 15, 2012
    24. gpshead
      assigned
      gpshead
      and unassigned
      warsaw
      on Mar 15, 2012

    25. python-dev commented on Mar 15, 2012

      @python-dev
      python-devmannequin
      on Mar 15, 2012
      Mannequin

      New changeset 04ff6e206b98 by Gregory P. Smith in branch '2.7':
      Fixes Issue bpo-14234: CVE-2012-0876: Randomize hashes of xml attributes
      http://hg.python.org/cpython/rev/04ff6e206b98

      New changeset ada6bfbeceb8 by Gregory P. Smith in branch '2.7':
      Fixes bpo-14234: fix for the previous commit, keep compilation when
      http://hg.python.org/cpython/rev/ada6bfbeceb8

    26. jimjjewett commented on Mar 15, 2012

      @jimjjewett
      jimjjewettmannequin
      on Mar 15, 2012
      Mannequin

      Looking at http://sourceforge.net/projects/expat/files/expat/2.1.0/, so long as XML_ATTR_INFO isn't defined at compile time, the changes are all considered bugfixes, and the XML_SetHashSalt is the only other changed API.

      Is a potential Denial of Service really worse than a crash, such as these fixed bugs:

      http://sourceforge.net/tracker/?func=detail&aid=2894085&group_id=10127&atid=110127

      http://sourceforge.net/tracker/?func=detail&aid=1990430&group_id=10127&atid=110127

    27. gpshead commented on Mar 16, 2012

      @gpshead
      gpshead
      on Mar 16, 2012
      Member

      I'm opening another issue to track updating the embedded copy of expat within Python.

      FWIW, Python 2.7 & 3.2 and later support a --with-system-expat option which is what I'd *hope* that any OS distro is building their Python with rather than using the older out of date embedded copy of expat (which appears to be derived from expat 2.0.0).

    28. python-dev commented on Mar 18, 2012

      @python-dev
      python-devmannequin
      on Mar 18, 2012
      Mannequin

      New changeset cf7337a49a07 by Georg Brandl in branch '3.2':
      Transplant from main repo d6c197edd99b: Fixes Issue bpo-14234: CVE-2012-0876: Randomize hashes of xml attributes
      http://hg.python.org/cpython/rev/cf7337a49a07

      New changeset d54508a86a5d by Gregory P. Smith in branch '3.2':
      Fixes bpo-14234: fix for the previous commit, keep compilation when
      http://hg.python.org/cpython/rev/d54508a86a5d

    29. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    Labels

    release-blockertopic-XML

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions