classification
Title: crashes in _elementtree due to unsafe decrefs of Element.text and Element.tail
Type: crash Stage: resolved
Components: XML Versions: Python 3.7, Python 3.6, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: Oren Milman, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2017-10-08 20:16 by Oren Milman, last changed 2017-10-11 13:44 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 3924 merged Oren Milman, 2017-10-08 21:09
PR 3945 merged python-dev, 2017-10-10 21:02
PR 3950 merged Oren Milman, 2017-10-11 10:27
Messages (6)
msg303917 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-10-08 20:16
The following code causes the interpreter to crash:
import xml.etree.ElementTree
class X:
    def __del__(self):
        elem.clear()
elem = xml.etree.ElementTree.Element('elem')
elem.text = X()
elem.clear()

This is because _elementtree_Element_clear_impl() decrefs self->text in an
unsafe manner.
For the same reason, but for self->tail, a crash would happen if we replaced
'elem.text = X()' with 'elem.tail = X()'.


Similarly, the following code also causes the interpreter to crash:
import xml.etree.ElementTree
class X:
    def __del__(self):
        elem.clear()
elem = xml.etree.ElementTree.Element('elem')
elem.text = X()
elem.text = X()

This is because element_text_setter() decrefs self->text in an unsafe manner.
element_tail_setter() does the same for self->tail, so again, if we replaced
'elem.text = X()' with 'elem.tail = X()', we would also get a crash.
msg304013 - (view) Author: Oren Milman (Oren Milman) * Date: 2017-10-10 05:34
As serhiy pointed out in a comment in PR 3924, setting self->text or self->tail to
NULL might lead to an assertion failure, so we should also prevent the following
assertion failure (and the similar one for tail):
import xml.etree.ElementTree
class X:
    def __del__(self):
        elem.text
        
elem = xml.etree.ElementTree.Element('elem')
elem.text = X()
elem.__setstate__({'tag': None}) # implicitly also set elem.text to None
msg304079 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-10 20:26
New changeset 39ecb9c71b6e55d8a61a710d0144231bd88f9ada by Serhiy Storchaka (Oren Milman) in branch 'master':
bpo-31728: Prevent crashes in _elementtree due to unsafe cleanup of Element.text and Element.tail (#3924)
https://github.com/python/cpython/commit/39ecb9c71b6e55d8a61a710d0144231bd88f9ada
msg304087 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-10 21:51
New changeset a8ac71d15f2a4aef83a8954a678cc12545ce517c by Serhiy Storchaka (Miss Islington (bot)) in branch '3.6':
[3.6] bpo-31728: Prevent crashes in _elementtree due to unsafe cleanup of Element.text and Element.tail (GH-3924) (#3945)
https://github.com/python/cpython/commit/a8ac71d15f2a4aef83a8954a678cc12545ce517c
msg304132 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-11 13:29
New changeset f15058a697de12b0efe6c7ebc38fe61a993bb5d5 by Serhiy Storchaka (Oren Milman) in branch '2.7':
[2.7] bpo-31728: Prevent crashes in _elementtree due to unsafe cleanup of Element.text and Element.tail (GH-3924) (#3950)
https://github.com/python/cpython/commit/f15058a697de12b0efe6c7ebc38fe61a993bb5d5
msg304135 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2017-10-11 13:44
Thank you Oren!
History
Date User Action Args
2017-10-11 13:44:11serhiy.storchakasetstatus: open -> closed
resolution: fixed
messages: + msg304135

stage: patch review -> resolved
2017-10-11 13:29:14serhiy.storchakasetmessages: + msg304132
2017-10-11 10:27:11Oren Milmansetstage: backport needed -> patch review
pull_requests: + pull_request3925
2017-10-11 08:43:19serhiy.storchakasetstage: patch review -> backport needed
versions: + Python 2.7, Python 3.6
2017-10-10 21:51:30serhiy.storchakasetmessages: + msg304087
2017-10-10 21:02:08python-devsetpull_requests: + pull_request3919
2017-10-10 20:26:27serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg304079
2017-10-10 05:34:07Oren Milmansetmessages: + msg304013
2017-10-08 21:09:29Oren Milmansetkeywords: + patch
stage: patch review
pull_requests: + pull_request3897
2017-10-08 20:16:27Oren Milmancreate