bpo-41994: Fix refcount issues in Python/import.c

Author: serhiy-storchaka

Include/cpython/import.h

@@ -13,8 +13,6 @@ PyAPI_FUNC(int) _PyImport_SetModuleString(const char *name, PyObject* module);
 PyAPI_FUNC(void) _PyImport_AcquireLock(void);
 PyAPI_FUNC(int) _PyImport_ReleaseLock(void);
 
-PyAPI_FUNC(PyObject *) _PyImport_FindExtensionObject(PyObject *, PyObject *);
-
 PyAPI_FUNC(int) _PyImport_FixupBuiltin(
     PyObject *mod,
     const char *name,            /* UTF-8 encoded string */

Include/internal/pycore_import.h

@@ -5,11 +5,6 @@
 extern "C" {
 #endif
 
-PyAPI_FUNC(PyObject *) _PyImport_FindBuiltin(
-    PyThreadState *tstate,
-    const char *name             /* UTF-8 encoded string */
-    );
-
 #ifdef HAVE_FORK
 extern PyStatus _PyImport_ReInitLock(void);
 #endif

Misc/NEWS.d/next/Core and Builtins/2020-10-10-14-16-03.bpo-41994.Xop8sV.rst

@@ -0,0 +1 @@
+Fixed possible leak in ``import`` when ``sys.modules`` is not a ``dict``.

Python/import.c

@@ -672,7 +672,7 @@ PyImport_GetMagicTag(void)
    modules.  A copy of the module's dictionary is stored by calling
    _PyImport_FixupExtensionObject() immediately after the module initialization
    function succeeds.  A copy can be retrieved from there by calling
-   _PyImport_FindExtensionObject().
+   import_find_extension().
 
    Modules which do support multiple initialization set their m_size
    field to a non-negative number (indicating the size of the
@@ -785,10 +785,14 @@ import_find_extension(PyThreadState *tstate, PyObject *name,
         if (mod == NULL)
             return NULL;
         mdict = PyModule_GetDict(mod);
-        if (mdict == NULL)
+        if (mdict == NULL) {
+            Py_DECREF(mod);
             return NULL;
-        if (PyDict_Update(mdict, def->m_base.m_copy))
+        }
+        if (PyDict_Update(mdict, def->m_base.m_copy)) {
+            Py_DECREF(mod);
             return NULL;
+        }
     }
     else {
         if (def->m_base.m_init == NULL)
@@ -800,10 +804,10 @@ import_find_extension(PyThreadState *tstate, PyObject *name,
             Py_DECREF(mod);
             return NULL;
         }
-        Py_DECREF(mod);
     }
     if (_PyState_AddModule(tstate, mod, def) < 0) {
         PyMapping_DelItem(modules, name);
+        Py_DECREF(mod);
         return NULL;
     }
 
@@ -815,31 +819,10 @@ import_find_extension(PyThreadState *tstate, PyObject *name,
     return mod;
 }
 
-PyObject *
-_PyImport_FindExtensionObject(PyObject *name, PyObject *filename)
-{
-    PyThreadState *tstate = _PyThreadState_GET();
-    return import_find_extension(tstate, name, filename);
-}
-
-
-PyObject *
-_PyImport_FindBuiltin(PyThreadState *tstate, const char *name)
-{
-    PyObject *res, *nameobj;
-    nameobj = PyUnicode_InternFromString(name);
-    if (nameobj == NULL)
-        return NULL;
-    res = import_find_extension(tstate, nameobj, nameobj);
-    Py_DECREF(nameobj);
-    return res;
-}
 
 /* Get the module object corresponding to a module name.
    First check the modules dictionary if there's one there,
-   if not, create a new one and insert it in the modules dictionary.
-   Because the former action is most common, THIS DOES NOT RETURN A
-   'NEW' REFERENCE! */
+   if not, create a new one and insert it in the modules dictionary. */
 
 static PyObject *
 import_add_module(PyThreadState *tstate, PyObject *name)
@@ -854,6 +837,7 @@ import_add_module(PyThreadState *tstate, PyObject *name)
     PyObject *m;
     if (PyDict_CheckExact(modules)) {
         m = PyDict_GetItemWithError(modules, name);
+        Py_XINCREF(m);
     }
     else {
         m = PyObject_GetItem(modules, name);
@@ -869,14 +853,14 @@ import_add_module(PyThreadState *tstate, PyObject *name)
     if (m != NULL && PyModule_Check(m)) {
         return m;
     }
+    Py_XDECREF(m);
     m = PyModule_NewObject(name);
     if (m == NULL)
         return NULL;
     if (PyObject_SetItem(modules, name, m) != 0) {
         Py_DECREF(m);
         return NULL;
     }
-    Py_DECREF(m); /* Yes, it still exists, in modules! */
 
     return m;
 }
@@ -885,7 +869,25 @@ PyObject *
 PyImport_AddModuleObject(PyObject *name)
 {
     PyThreadState *tstate = _PyThreadState_GET();
-    return import_add_module(tstate, name);
+    PyObject *mod = import_add_module(tstate, name);
+    if (mod) {
+        if (Py_REFCNT(mod) == 1) {
+            /* This check does not preven an undefined behavior in the
+             * following code, because the module can have references
+             * to itself. */
+            PyErr_SetString(PyExc_RuntimeError, "Unexpected zero reference count");
+            Py_DECREF(mod);
+            return NULL;
+        }
+        Py_DECREF(mod);
+        if (Py_REFCNT(mod) == 0) {
+            /* Strictly speaking, this is an undefined behafior, and
+             * the above check can crash. */
+            PyErr_SetString(PyExc_RuntimeError, "Unexpected zero reference count");
+            return NULL;
+        }
+    }
+    return mod; /* borrowed reference */
 }
 
 
@@ -1007,7 +1009,7 @@ static PyObject *
 module_dict_for_exec(PyThreadState *tstate, PyObject *name)
 {
     _Py_IDENTIFIER(__builtins__);
-    PyObject *m, *d = NULL;
+    PyObject *m, *d;
 
     m = import_add_module(tstate, name);
     if (m == NULL)
@@ -1021,11 +1023,14 @@ module_dict_for_exec(PyThreadState *tstate, PyObject *name)
                               PyEval_GetBuiltins()) != 0)
         {
             remove_module(tstate, name);
+            Py_DECREF(m);
             return NULL;
         }
     }
 
-    return d;  /* Return a borrowed reference. */
+    Py_INCREF(d);
+    Py_DECREF(m);
+    return d;
 }
 
 static PyObject *
@@ -1069,8 +1074,10 @@ PyImport_ExecCodeModuleObject(PyObject *name, PyObject *co, PyObject *pathname,
     }
     external = PyObject_GetAttrString(tstate->interp->importlib,
                                       "_bootstrap_external");
-    if (external == NULL)
+    if (external == NULL) {
+        Py_DECREF(d);
         return NULL;
+    }
     res = _PyObject_CallMethodIdObjArgs(external,
                                         &PyId__fix_up_module,
                                         d, name, pathname, cpathname, NULL);
@@ -1079,6 +1086,7 @@ PyImport_ExecCodeModuleObject(PyObject *name, PyObject *co, PyObject *pathname,
         Py_DECREF(res);
         res = exec_code_in_module(tstate, name, d, co);
     }
+    Py_DECREF(d);
     return res;
 }
 
@@ -1263,10 +1271,9 @@ _imp_create_builtin(PyObject *module, PyObject *spec)
         return NULL;
     }
 
-    mod = _PyImport_FindExtensionObject(name, name);
+    mod = import_find_extension(tstate, name, name);
     if (mod || _PyErr_Occurred(tstate)) {
         Py_DECREF(name);
-        Py_XINCREF(mod);
         return mod;
     }
 
@@ -1429,10 +1436,12 @@ PyImport_ImportFrozenModuleObject(PyObject *name)
         d = PyModule_GetDict(m);
         l = PyList_New(0);
         if (l == NULL) {
+            Py_DECREF(m);
             goto err_return;
         }
         err = PyDict_SetItemString(d, "__path__", l);
         Py_DECREF(l);
+        Py_DECREF(m);
         if (err != 0)
             goto err_return;
     }
@@ -1442,10 +1451,12 @@ PyImport_ImportFrozenModuleObject(PyObject *name)
     }
     m = exec_code_in_module(tstate, name, d, co);
     if (m == NULL) {
+        Py_DECREF(d);
         goto err_return;
     }
     Py_DECREF(co);
     Py_DECREF(m);
+    Py_DECREF(d);
     return 1;
 
 err_return:
@@ -2135,17 +2146,14 @@ _imp_init_frozen_impl(PyObject *module, PyObject *name)
 {
     PyThreadState *tstate = _PyThreadState_GET();
     int ret;
-    PyObject *m;
 
     ret = PyImport_ImportFrozenModuleObject(name);
     if (ret < 0)
         return NULL;
     if (ret == 0) {
         Py_RETURN_NONE;
     }
-    m = import_add_module(tstate, name);
-    Py_XINCREF(m);
-    return m;
+    return import_add_module(tstate, name);
 }
 
 /*[clinic input]
@@ -2269,11 +2277,11 @@ _imp_create_dynamic_impl(PyObject *module, PyObject *spec, PyObject *file)
         return NULL;
     }
 
-    mod = _PyImport_FindExtensionObject(name, path);
+    PyThreadState *tstate = _PyThreadState_GET();
+    mod = import_find_extension(tstate, name, path);
     if (mod != NULL || PyErr_Occurred()) {
         Py_DECREF(name);
         Py_DECREF(path);
-        Py_XINCREF(mod);
         return mod;
     }