Skip to content

[3.4] bpo-34623: Use XML_SetHashSalt in _elementtree #9953

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.

Sign up for GitHub

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

Jump to bottom
Merged
larryhastings merged 2 commits into from
Feb 25, 2019
Merged

[3.4] bpo-34623: Use XML_SetHashSalt in _elementtree #9953

larryhastings merged 2 commits into from
Feb 25, 2019

Conversation

@stratakis
Copy link
Contributor

@stratakis stratakis commented Oct 18, 2018 •
edited by bedevere-bot
Loading

Backport for the 3.4 branch.

https://bugs.python.org/issue34623

miss-islington and others added 2 commits October 18, 2018 16:54
@miss-islington @tiran @stratakis
bpo-34623: Use XML_SetHashSalt in _elementtree (pythonGH-9146)
d24304f 
The C accelerated _elementtree module now initializes hash randomization
salt from _Py_HashSecret instead of libexpat's default CPRNG.

Signed-off-by: Christian Heimes <[email protected]>

https://bugs.python.org/issue34623
(cherry picked from commit cb5778f)

Co-authored-by: Christian Heimes <[email protected]>
@miss-islington @stratakis
[3.4] bpo-34623: Mention CVE-2018-14647 in news entry (pythonGH-9482) (…
a535553 
…pythonGH-9489)

https://bugs.python.org/issue34623
(cherry picked from commit 026337a)

Co-authored-by: Christian Heimes <[email protected]>

https://bugs.python.org/issue34623
stratakis
stratakis commented Oct 18, 2018
View reviewed changes
Modules/_elementtree.c
}
/* expat < 2.1.0 has no XML_SetHashSalt() */
if (EXPAT(SetHashSalt) != NULL) {
EXPAT(SetHashSalt)(self_xp->parser,
Copy link
Contributor Author

@stratakis stratakis Oct 18, 2018

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the only line that has been modified from the previous PR's. Namely it's self_xp->parser on 3.4. On the other branches it's self->parser due to argumentclinication.

vstinner
vstinner approved these changes Oct 18, 2018
View reviewed changes
Copy link
Member

@vstinner vstinner left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM.

@tiran: Would you mind to review it as well?

@serhiy-storchaka serhiy-storchaka added the type-security A security issue label Oct 18, 2018
@vstinner
Copy link
Member

vstinner commented Oct 24, 2018

Oh, Travis CI failed on a random failure:

======================================================================
ERROR: test_ignore (test.test_multiprocessing_forkserver.TestIgnoreEINTR)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/home/travis/build/python/cpython/Lib/test/_test_multiprocessing.py", line 3623, in test_ignore
    os.kill(p.pid, signal.SIGUSR1)
ProcessLookupError: [Errno 3] No such process

I scheduled a new job.

@vstinner vstinner closed this Jan 21, 2019
@vstinner vstinner reopened this Jan 21, 2019
@vstinner
Copy link
Member

vstinner commented Jan 21, 2019

I closed/reopened the PR to trigger a new Travis CI job.

@vstinner
Copy link
Member

vstinner commented Jan 23, 2019

@larryhastings: Hi Larry, would you mind to merge this security fix?

@larryhastings larryhastings merged commit d16eaf3 into python:3.4 Feb 25, 2019
@bedevere-bot
Copy link

bedevere-bot commented Feb 25, 2019

@larryhastings: Please replace # with GH- in the commit message next time. Thanks!

@larryhastings
Copy link
Contributor

larryhastings commented Feb 25, 2019

Thank for the backported fix!

@stratakis stratakis deleted the 3.4 branch June 18, 2020 13:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@vstinner vstinner vstinner approved these changes

@tiran tiran Awaiting requested review from tiran

Assignees

@larryhastings larryhastings

Labels

type-security A security issue

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

7 participants

@stratakis @vstinner @bedevere-bot @larryhastings @serhiy-storchaka @the-knights-who-say-ni @miss-islington