From: Donghee Na <donghee.na@python.org>
Date: Thu, 22 Aug 2024 16:37:26 +0000 (+0900)
Subject: [3.13] gh-123083: Fix a potential use-after-free in ``STORE_ATTR_WITH… (#123235)
X-Git-Tag: v3.13.0rc2~117
X-Git-Url: http://git.ipfire.org/gitweb.cgi?a=commitdiff_plain;h=6cd67e413b9a6c5c81466c4e516bca6430d7297e;p=thirdparty%2FPython%2Fcpython.git

[3.13] gh-123083: Fix a potential use-after-free in ``STORE_ATTR_WITH… (#123235)

[3.13] gh-123083: Fix a potential use-after-free in ``STORE_ATTR_WITH_HINT`` (gh-123092)
(cherry picked from commit 297f2e093ec95800ae2184330b8408c875523467)
---

diff --git a/Lib/test/test_dict.py b/Lib/test/test_dict.py
index e5dba7cdc570..4030716efb51 100644
--- a/Lib/test/test_dict.py
+++ b/Lib/test/test_dict.py
@@ -1476,6 +1476,24 @@ class DictTest(unittest.TestCase):
         gc.collect()
         self.assertTrue(gc.is_tracked(next(it)))
 
+    def test_store_evilattr(self):
+        class EvilAttr:
+            def __init__(self, d):
+                self.d = d
+
+            def __del__(self):
+                if 'attr' in self.d:
+                    del self.d['attr']
+                gc.collect()
+
+        class Obj:
+            pass
+
+        obj = Obj()
+        obj.__dict__ = {}
+        for _ in range(10):
+            obj.attr = EvilAttr(obj.__dict__)
+
     def test_str_nonstr(self):
         # cpython uses a different lookup function if the dict only contains
         # `str` keys. Make sure the unoptimized path is used when a non-`str`
diff --git a/Misc/NEWS.d/next/Core_and_Builtins/2024-08-17-17-26-25.gh-issue-123083.9xWLJ-.rst b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-17-17-26-25.gh-issue-123083.9xWLJ-.rst
new file mode 100644
index 000000000000..edc3f1ab6a8e
--- /dev/null
+++ b/Misc/NEWS.d/next/Core_and_Builtins/2024-08-17-17-26-25.gh-issue-123083.9xWLJ-.rst
@@ -0,0 +1 @@
+Fix a potential use-after-free in ``STORE_ATTR_WITH_HINT``.
diff --git a/Objects/dictobject.c b/Objects/dictobject.c
index 05600772db70..6586e0c5c27d 100644
--- a/Objects/dictobject.c
+++ b/Objects/dictobject.c
@@ -1762,6 +1762,8 @@ insert_split_value(PyInterpreterState *interp, PyDictObject *mp, PyObject *key,
         uint64_t new_version = _PyDict_NotifyEvent(interp, PyDict_EVENT_MODIFIED, mp, key, value);
         STORE_SPLIT_VALUE(mp, ix, Py_NewRef(value));
         mp->ma_version_tag = new_version;
+        // old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault,
+        // when dict only holds the strong reference to value in ep->me_value.
         Py_DECREF(old_value);
     }
     ASSERT_CONSISTENT(mp);
diff --git a/Python/bytecodes.c b/Python/bytecodes.c
index cac9f67d7f21..c222fcdb22f8 100644
--- a/Python/bytecodes.c
+++ b/Python/bytecodes.c
@@ -2170,14 +2170,15 @@ dummy_func(
                 new_version = _PyDict_NotifyEvent(tstate->interp, PyDict_EVENT_MODIFIED, dict, name, value);
                 ep->me_value = value;
             }
-            Py_DECREF(old_value);
-            STAT_INC(STORE_ATTR, hit);
             /* Ensure dict is GC tracked if it needs to be */
             if (!_PyObject_GC_IS_TRACKED(dict) && _PyObject_GC_MAY_BE_TRACKED(value)) {
                 _PyObject_GC_TRACK(dict);
             }
-            /* PEP 509 */
-            dict->ma_version_tag = new_version;
+            dict->ma_version_tag = new_version; // PEP 509
+            // old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault,
+            // when dict only holds the strong reference to value in ep->me_value.
+            Py_DECREF(old_value);
+            STAT_INC(STORE_ATTR, hit);
             Py_DECREF(owner);
         }
 
diff --git a/Python/generated_cases.c.h b/Python/generated_cases.c.h
index fe42e20f4393..dd334044d559 100644
--- a/Python/generated_cases.c.h
+++ b/Python/generated_cases.c.h
@@ -5608,14 +5608,15 @@
                 new_version = _PyDict_NotifyEvent(tstate->interp, PyDict_EVENT_MODIFIED, dict, name, value);
                 ep->me_value = value;
             }
-            Py_DECREF(old_value);
-            STAT_INC(STORE_ATTR, hit);
             /* Ensure dict is GC tracked if it needs to be */
             if (!_PyObject_GC_IS_TRACKED(dict) && _PyObject_GC_MAY_BE_TRACKED(value)) {
                 _PyObject_GC_TRACK(dict);
             }
-            /* PEP 509 */
-            dict->ma_version_tag = new_version;
+            dict->ma_version_tag = new_version; // PEP 509
+            // old_value should be DECREFed after GC track checking is done, if not, it could raise a segmentation fault,
+            // when dict only holds the strong reference to value in ep->me_value.
+            Py_DECREF(old_value);
+            STAT_INC(STORE_ATTR, hit);
             Py_DECREF(owner);
             stack_pointer += -2;
             DISPATCH();