From 59df735ac8a25024daae91bc7cf36771fcdb28b7 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 4 Dec 2021 20:08:46 +0200 Subject: [PATCH 1/2] bpo-27946: Fix possible crash in ElementTree.Element Getting an attribute via attrib.get() simultaneously with replacing the attrib dict can lead to access to deallocated dict. --- Lib/test/test_xml_etree_c.py | 12 ++++++++++ .../2021-12-04-20-08-42.bpo-27946.-Vuarf.rst | 3 +++ Modules/_elementtree.c | 23 ++++++++----------- 3 files changed, 25 insertions(+), 13 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst diff --git a/Lib/test/test_xml_etree_c.py b/Lib/test/test_xml_etree_c.py index e68613bb910dd4..bec8208571902e 100644 --- a/Lib/test/test_xml_etree_c.py +++ b/Lib/test/test_xml_etree_c.py @@ -169,6 +169,18 @@ def test_xmlpullparser_leaks(self): del parser support.gc_collect() + def test_dict_disappearing_during_get_item(self): + # test fix for seg fault reported in issue 27946 + class X: + def __hash__(self): + e.attrib = {} # this frees e->extra->attrib + [{i: i} for i in range(1000)] # exhaust the dict keys cache + return 13 + + e = cET.Element("elem", {1: 2}) + r = e.get(X()) + self.assertIsNone(r) + @unittest.skipUnless(cET, 'requires _elementtree') class TestAliasWorking(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst b/Misc/NEWS.d/next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst new file mode 100644 index 00000000000000..0a0d54e81a05fd --- /dev/null +++ b/Misc/NEWS.d/next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst @@ -0,0 +1,3 @@ +Fix possible crash when get an attribute of +class:`xml.etree.ElementTree.Element` simultaneously with +replacing the ``attrib`` dict. diff --git a/Modules/_elementtree.c b/Modules/_elementtree.c index b4528a90b3e095..9dadeef7129384 100644 --- a/Modules/_elementtree.c +++ b/Modules/_elementtree.c @@ -1393,22 +1393,19 @@ _elementtree_Element_get_impl(ElementObject *self, PyObject *key, PyObject *default_value) /*[clinic end generated code: output=523c614142595d75 input=ee153bbf8cdb246e]*/ { - PyObject* value; - - if (!self->extra || !self->extra->attrib) - value = default_value; - else { - value = PyDict_GetItemWithError(self->extra->attrib, key); - if (!value) { - if (PyErr_Occurred()) { - return NULL; - } - value = default_value; + if (self->extra && self->extra->attrib) { + PyObject *attrib = self->extra->attrib; + Py_INCREF(attrib); + PyObject *value = PyDict_GetItemWithError(attrib, key); + Py_XINCREF(value); + Py_DECREF(attrib); + if (value != NULL || PyErr_Occurred()) { + return value; } } - Py_INCREF(value); - return value; + Py_INCREF(default_value); + return default_value; } static PyObject * From bd34a0878688b5b3c309714e3f098bd143e05507 Mon Sep 17 00:00:00 2001 From: Serhiy Storchaka Date: Sat, 4 Dec 2021 21:34:33 +0200 Subject: [PATCH 2/2] Update Misc/NEWS.d/next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst Co-authored-by: Irit Katriel <1055913+iritkatriel@users.noreply.github.com> --- .../next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Misc/NEWS.d/next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst b/Misc/NEWS.d/next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst index 0a0d54e81a05fd..0378efca746bbf 100644 --- a/Misc/NEWS.d/next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst +++ b/Misc/NEWS.d/next/Library/2021-12-04-20-08-42.bpo-27946.-Vuarf.rst @@ -1,3 +1,3 @@ -Fix possible crash when get an attribute of +Fix possible crash when getting an attribute of class:`xml.etree.ElementTree.Element` simultaneously with replacing the ``attrib`` dict.