Skip to content

Commit 4b45e07

Browse files
committed
gh-144984: Fix crash in ExternalEntityParserCreate() error paths
When ExternalEntityParserCreate() hits an error path (allocation failure), Py_DECREF(new_parser) triggers xmlparse_dealloc() on a partially-initialized object: 1. handlers is NULL, so clear_handlers dereferences NULL (SEGV). 2. Py_CLEAR(parent) in dealloc already decrements the parent's refcount, so the explicit Py_DECREF(self) is a double-decrement. Fix by adding a NULL guard in clear_handlers and setting parent to NULL before Py_DECREF(new_parser) in each error path so that dealloc does not over-decrement the parent's refcount.
1 parent e84a2cc commit 4b45e07

File tree

3 files changed

+38
-0
lines changed

3 files changed

+38
-0
lines changed

Lib/test/test_pyexpat.py

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -824,6 +824,35 @@ def test_parent_parser_outlives_its_subparsers__chain(self):
824824
del subparser
825825

826826

827+
class ExternalEntityParserCreateErrorTest(unittest.TestCase):
828+
"""ExternalEntityParserCreate error paths should not crash or leak
829+
refcounts on the parent parser.
830+
831+
See https://github.com/python/cpython/issues/144984.
832+
"""
833+
834+
def test_error_path_no_crash(self):
835+
# When an allocation inside ExternalEntityParserCreate fails,
836+
# the partially-initialized subparser is deallocated. This
837+
# must not dereference NULL handlers or double-decrement the
838+
# parent parser's refcount.
839+
_testcapi = import_helper.import_module('_testcapi')
840+
parser = expat.ParserCreate()
841+
parser.buffer_text = True
842+
rc_before = sys.getrefcount(parser)
843+
844+
_testcapi.set_nomemory(1, 10)
845+
try:
846+
parser.ExternalEntityParserCreate(None)
847+
except MemoryError:
848+
pass
849+
finally:
850+
_testcapi.remove_mem_hooks()
851+
852+
rc_after = sys.getrefcount(parser)
853+
self.assertEqual(rc_after, rc_before)
854+
855+
827856
class ReparseDeferralTest(unittest.TestCase):
828857
def test_getter_setter_round_trip(self):
829858
parser = expat.ParserCreate()
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix crash in :meth:`xml.parsers.expat.xmlparser.ExternalEntityParserCreate`
2+
when an allocation fails. The error paths could dereference NULL ``handlers``
3+
and double-decrement the parent parser's reference count.

Modules/pyexpat.c

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1097,12 +1097,14 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
10971097
if (self->buffer != NULL) {
10981098
new_parser->buffer = PyMem_Malloc(new_parser->buffer_size);
10991099
if (new_parser->buffer == NULL) {
1100+
new_parser->parent = NULL;
11001101
Py_DECREF(new_parser);
11011102
Py_DECREF(self);
11021103
return PyErr_NoMemory();
11031104
}
11041105
}
11051106
if (!new_parser->itself) {
1107+
new_parser->parent = NULL;
11061108
Py_DECREF(new_parser);
11071109
Py_DECREF(self);
11081110
return PyErr_NoMemory();
@@ -1117,6 +1119,7 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
11171119

11181120
new_parser->handlers = PyMem_New(PyObject *, i);
11191121
if (!new_parser->handlers) {
1122+
new_parser->parent = NULL;
11201123
Py_DECREF(new_parser);
11211124
Py_DECREF(self);
11221125
return PyErr_NoMemory();
@@ -2489,6 +2492,9 @@ PyInit_pyexpat(void)
24892492
static void
24902493
clear_handlers(xmlparseobject *self, int initial)
24912494
{
2495+
if (self->handlers == NULL) {
2496+
return;
2497+
}
24922498
for (size_t i = 0; handler_info[i].name != NULL; i++) {
24932499
if (initial) {
24942500
self->handlers[i] = NULL;

0 commit comments

Comments
 (0)