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

_elementtree.c doesn't call XML_SetHashSalt() #78804

Closed
tiran opened this issue Sep 10, 2018 · 20 comments
Closed

_elementtree.c doesn't call XML_SetHashSalt() #78804

tiran opened this issue Sep 10, 2018 · 20 comments
Assignees

Comments

@tiran
Copy link
Member

tiran commented Sep 10, 2018

BPO 34623
Nosy @vstinner, @larryhastings, @tiran, @benjaminp, @ned-deily, @mcepl, @stratakis, @miss-islington
PRs
  • bpo-34623: Use XML_SetHashSalt in _elementtree #9146
  • [3.7] bpo-34623: Use XML_SetHashSalt in _elementtree (GH-9146) #9392
  • [3.6] bpo-34623: Use XML_SetHashSalt in _elementtree (GH-9146) #9393
  • [2.7] bpo-34623: Use XML_SetHashSalt in _elementtree (GH-9146) #9394
  • bpo-34623: Mention CVE-2018-14647 in news entry #9482
  • [3.7] bpo-34623: Mention CVE-2018-14647 in news entry (GH-9482) #9488
  • [3.6] bpo-34623: Mention CVE-2018-14647 in news entry (GH-9482) #9489
  • [2.7] bpo-34623: Mention CVE-2018-14647 in news entry (GH-9482) #9490
  • [3.5] bpo-34623: Use XML_SetHashSalt in _elementtree #9933
  • [3.4] bpo-34623: Use XML_SetHashSalt in _elementtree #9953
  • Files
  • CVE-2018-14647_XML_SetHashSalt-in_elementtree.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/larryhastings'
    closed_at = <Date 2019-03-01.07:38:24.620>
    created_at = <Date 2018-09-10.22:47:44.446>
    labels = ['type-security', 'expert-XML', 'release-blocker']
    title = "_elementtree.c doesn't call XML_SetHashSalt()"
    updated_at = <Date 2019-05-10.18:10:05.677>
    user = 'https://github.com/tiran'

    bugs.python.org fields:

    activity = <Date 2019-05-10.18:10:05.677>
    actor = 'ned.deily'
    assignee = 'larry'
    closed = True
    closed_date = <Date 2019-03-01.07:38:24.620>
    closer = 'larry'
    components = ['XML']
    creation = <Date 2018-09-10.22:47:44.446>
    creator = 'christian.heimes'
    dependencies = []
    files = ['48077']
    hgrepos = []
    issue_num = 34623
    keywords = ['patch']
    message_count = 20.0
    messages = ['324954', '324961', '324963', '324966', '325633', '325634', '325636', '325637', '325638', '325966', '325967', '326019', '326069', '326071', '326072', '326073', '327758', '334369', '336551', '336552']
    nosy_count = 8.0
    nosy_names = ['vstinner', 'larry', 'christian.heimes', 'benjamin.peterson', 'ned.deily', 'mcepl', 'cstratak', 'miss-islington']
    pr_nums = ['9146', '9392', '9393', '9394', '9482', '9488', '9489', '9490', '9933', '9953']
    priority = 'release blocker'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'security'
    url = 'https://bugs.python.org/issue34623'
    versions = ['Python 3.4', 'Python 3.5']

    @tiran
    Copy link
    Member Author

    tiran commented Sep 10, 2018

    The pyexpat module calls XML_SetHashSalt(self->itself, (unsigned long)_Py_HashSecret.expat.hashsalt) to initialize the salt for hash randomization of the XML_Parser struct. The _elementree C accelerator doesn't call XML_SetHashSalt().

    It's not a security issue with recent versions of libexpat. The library initializes the salt from a good entry source by default.

    @tiran tiran added type-bug An unexpected behavior, bug, or error 3.8 only security fixes topic-XML labels Sep 10, 2018
    @benjaminp
    Copy link
    Contributor

    Note we compile expat with -DXML_POOR_ENTROPY on the assumption that Python always initializes the entropy itself.

    @vstinner
    Copy link
    Member

    Note we compile expat with -DXML_POOR_ENTROPY on the assumption that Python always initializes the entropy itself.

    Oh. I forgot this thing. So it seems like we have to backport this change to 2.7, 3.6 and newer versions.

    What about Python 3.4 and 3.5? Python 3.5 has a copy of libexpat 2.2.0, but setup.py doesn't build expat with XML_POOR_ENTROPY=1.

    @tiran
    Copy link
    Member Author

    tiran commented Sep 11, 2018

    Dang, it's a security bug after all. :(

    3.5 has 2.2.4, so it's fine. 2.2.2 had a bug in salt initialization.

    @tiran tiran added 3.7 (EOL) end of life type-security A security issue and removed type-bug An unexpected behavior, bug, or error labels Sep 11, 2018
    @miss-islington
    Copy link
    Contributor

    New changeset cb5778f by Miss Islington (bot) (Christian Heimes) in branch 'master':
    bpo-34623: Use XML_SetHashSalt in _elementtree (GH-9146)
    cb5778f

    @tiran
    Copy link
    Member Author

    tiran commented Sep 18, 2018

    Since it's a security fix, the change should land in 3.4 and 3.5, too.

    @miss-islington
    Copy link
    Contributor

    New changeset 470a435 by Miss Islington (bot) in branch '3.7':
    bpo-34623: Use XML_SetHashSalt in _elementtree (GH-9146)
    470a435

    @miss-islington
    Copy link
    Contributor

    New changeset 18b20ba by Miss Islington (bot) (Christian Heimes) in branch '2.7':
    [2.7] bpo-34623: Use XML_SetHashSalt in _elementtree (GH-9146) (GH-9394)
    18b20ba

    @miss-islington
    Copy link
    Contributor

    New changeset f7666e8 by Miss Islington (bot) in branch '3.6':
    bpo-34623: Use XML_SetHashSalt in _elementtree (GH-9146)
    f7666e8

    @tiran
    Copy link
    Member Author

    tiran commented Sep 21, 2018

    The bug affects multiple platforms. libexpat's expat.h uses slightly different autoconf macro names than pyconfig.h. Therefore only platforms that have either HAVE_GETRANDOM or _WIN32 defined, use a proper CSPRNG to seed the hash salt.

    Since HAVE_SYSCALL_GETRANDOM, HAVE_ARC4RANDOM_BUF, HAVE_ARC4RANDOM, or XML_DEV_URANDOM are never defined by Python's pyconfig.h, older Linux platforms, any BSD, and any other Unix platform with /dev/urandom fall back to a weak Mersenne Twister-like RNG with gettimeofday().tv_usec and getpid() as seed.

    @tiran
    Copy link
    Member Author

    tiran commented Sep 21, 2018

    I have contected Red Hat product security to request a CVE for the issue.

    @tiran
    Copy link
    Member Author

    tiran commented Sep 21, 2018

    CVE-2018-14647 was assigned to this issue.

    @miss-islington
    Copy link
    Contributor

    New changeset 026337a by Miss Islington (bot) (Christian Heimes) in branch 'master':
    bpo-34623: Mention CVE-2018-14647 in news entry (GH-9482)
    026337a

    @miss-islington
    Copy link
    Contributor

    New changeset 10be1d3 by Miss Islington (bot) in branch '2.7':
    [2.7] bpo-34623: Mention CVE-2018-14647 in news entry (GH-9482) (GH-9490)
    10be1d3

    @miss-islington
    Copy link
    Contributor

    New changeset d1b336e by Miss Islington (bot) in branch '3.6':
    [3.6] bpo-34623: Mention CVE-2018-14647 in news entry (GH-9482) (GH-9489)
    d1b336e

    @miss-islington
    Copy link
    Contributor

    New changeset 5c3d8b2 by Miss Islington (bot) in branch '3.7':
    [3.7] bpo-34623: Mention CVE-2018-14647 in news entry (GH-9482) (GH-9488)
    5c3d8b2

    @stratakis
    Copy link
    Mannequin

    stratakis mannequin commented Oct 15, 2018

    Will this change be backported to 3.5 and 3.4? It applied cleanly on both however on 3.4 there is a test failure:

    ======================================================================
    ERROR: test_del_attribute (test.test_xml_etree_c.MiscTests)
    ----------------------------------------------------------------------

    Traceback (most recent call last):
      File "/builddir/build/BUILD/Python-3.4.9/Lib/test/test_xml_etree_c.py", line 26, in test_del_attribute
        element = cET.Element('tag')
    AttributeError: 'NoneType' object has no attribute 'Element'

    @serhiy-storchaka serhiy-storchaka added release-blocker and removed 3.7 (EOL) end of life 3.8 only security fixes labels Oct 18, 2018
    @mcepl
    Copy link
    Mannequin

    mcepl mannequin commented Jan 25, 2019

    Will this change be backported to 3.5 and 3.4? It applied cleanly on both however on 3.4 there is a test failure:

    It actually haven't applied cleanly to me on Python 3.4.6 (SLE-12 package). Apparently self->parser has to be changed into self_xp->parser. Then all tests passed for me.

    If any Linux maintainer wants to take this patch.

    @larryhastings
    Copy link
    Contributor

    New changeset d16eaf3 by larryhastings (stratakis) in branch '3.4':
    [3.4] bpo-34623: Use XML_SetHashSalt in _elementtree (bpo-9953)
    d16eaf3

    @larryhastings
    Copy link
    Contributor

    New changeset 41b48e7 by larryhastings (stratakis) in branch '3.5':
    [3.5] bpo-34623: Use XML_SetHashSalt in _elementtree (bpo-9933)
    41b48e7

    @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
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants