From: Ramin Farajpour Cami <ramin.blackhat@gmail.com>
Date: Tue, 7 Apr 2026 10:22:22 +0000 (+0330)
Subject: [3.13] gh-144984: Fix crash in Expat's `ExternalEntityParserCreate` error paths ... 
X-Git-Tag: v3.13.13~5
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=4c8d6f4f6857657873a48ad88c8b619d7c0b0f6b;p=thirdparty%2FPython%2Fcpython.git

[3.13] gh-144984: Fix crash in Expat's `ExternalEntityParserCreate` error paths (GH-144992) (#146142)

* gh-144984: Fix crash in Expat's `ExternalEntityParserCreate` error paths (#144992)

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>

* gh-144984: Skip test under tracerefs (GH-146218)

---------

Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
Co-authored-by: Petr Viktorin <encukou@gmail.com>
---

diff --git a/Lib/test/test_pyexpat.py b/Lib/test/test_pyexpat.py
index cceeff4341e8..2309353503f8 100644
--- a/Lib/test/test_pyexpat.py
+++ b/Lib/test/test_pyexpat.py
@@ -827,6 +827,45 @@ class ParentParserLifetimeTest(unittest.TestCase):
         del subparser
 
 
+class ExternalEntityParserCreateErrorTest(unittest.TestCase):
+    """ExternalEntityParserCreate error paths should not crash or leak
+    refcounts on the parent parser.
+
+    See https://github.com/python/cpython/issues/144984.
+    """
+
+    @classmethod
+    def setUpClass(cls):
+        cls.testcapi = import_helper.import_module('_testcapi')
+
+    @unittest.skipIf(support.Py_TRACE_REFS,
+                     'Py_TRACE_REFS conflicts with testcapi.set_nomemory')
+    def test_error_path_no_crash(self):
+        # When an allocation inside ExternalEntityParserCreate fails,
+        # the partially-initialized subparser is deallocated.  This
+        # must not dereference NULL handlers or double-decrement the
+        # parent parser's refcount.
+        parser = expat.ParserCreate()
+        parser.buffer_text = True
+        rc_before = sys.getrefcount(parser)
+
+        # We avoid self.assertRaises(MemoryError) here because the
+        # context manager itself needs memory allocations that fail
+        # while the nomemory hook is active.
+        self.testcapi.set_nomemory(1, 10)
+        raised = False
+        try:
+            parser.ExternalEntityParserCreate(None)
+        except MemoryError:
+            raised = True
+        finally:
+            self.testcapi.remove_mem_hooks()
+        self.assertTrue(raised, "MemoryError not raised")
+
+        rc_after = sys.getrefcount(parser)
+        self.assertEqual(rc_after, rc_before)
+
+
 class ReparseDeferralTest(unittest.TestCase):
     def test_getter_setter_round_trip(self):
         parser = expat.ParserCreate()
diff --git a/Misc/NEWS.d/next/Library/2026-02-19-12-00-00.gh-issue-144984.b93995c982.rst b/Misc/NEWS.d/next/Library/2026-02-19-12-00-00.gh-issue-144984.b93995c982.rst
new file mode 100644
index 000000000000..66e07dc3098c
--- /dev/null
+++ b/Misc/NEWS.d/next/Library/2026-02-19-12-00-00.gh-issue-144984.b93995c982.rst
@@ -0,0 +1,3 @@
+Fix crash in :meth:`xml.parsers.expat.xmlparser.ExternalEntityParserCreate`
+when an allocation fails. The error paths could dereference NULL ``handlers``
+and double-decrement the parent parser's reference count.
diff --git a/Modules/pyexpat.c b/Modules/pyexpat.c
index 1eeff2ea91a3..65802fca221b 100644
--- a/Modules/pyexpat.c
+++ b/Modules/pyexpat.c
@@ -1045,11 +1045,6 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
         return NULL;
     }
 
-    // The new subparser will make use of the parent XML_Parser inside of Expat.
-    // So we need to take subparsers into account with the reference counting
-    // of their parent parser.
-    Py_INCREF(self);
-
     new_parser->buffer_size = self->buffer_size;
     new_parser->buffer_used = 0;
     new_parser->buffer = NULL;
@@ -1059,7 +1054,10 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
     new_parser->ns_prefixes = self->ns_prefixes;
     new_parser->itself = XML_ExternalEntityParserCreate(self->itself, context,
                                                         encoding);
-    new_parser->parent = (PyObject *)self;
+    // The new subparser will make use of the parent XML_Parser inside of Expat.
+    // So we need to take subparsers into account with the reference counting
+    // of their parent parser.
+    new_parser->parent = Py_NewRef(self);
     new_parser->handlers = 0;
     new_parser->intern = Py_XNewRef(self->intern);
 
@@ -1067,13 +1065,11 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
         new_parser->buffer = PyMem_Malloc(new_parser->buffer_size);
         if (new_parser->buffer == NULL) {
             Py_DECREF(new_parser);
-            Py_DECREF(self);
             return PyErr_NoMemory();
         }
     }
     if (!new_parser->itself) {
         Py_DECREF(new_parser);
-        Py_DECREF(self);
         return PyErr_NoMemory();
     }
 
@@ -1086,7 +1082,6 @@ pyexpat_xmlparser_ExternalEntityParserCreate_impl(xmlparseobject *self,
     new_parser->handlers = PyMem_New(PyObject *, i);
     if (!new_parser->handlers) {
         Py_DECREF(new_parser);
-        Py_DECREF(self);
         return PyErr_NoMemory();
     }
     clear_handlers(new_parser, 1);
@@ -2333,11 +2328,13 @@ PyInit_pyexpat(void)
 static void
 clear_handlers(xmlparseobject *self, int initial)
 {
-    int i = 0;
-
-    for (; handler_info[i].name != NULL; i++) {
-        if (initial)
+    if (self->handlers == NULL) {
+        return;
+    }
+    for (size_t i = 0; handler_info[i].name != NULL; i++) {
+        if (initial) {
             self->handlers[i] = NULL;
+        }
         else {
             Py_CLEAR(self->handlers[i]);
             handler_info[i].setter(self->itself, NULL);