diff --git a/Lib/test/test_os/test_os.py b/Lib/test/test_os/test_os.py index ddb8a63095bce5..67ab945101149d 100644 --- a/Lib/test/test_os/test_os.py +++ b/Lib/test/test_os/test_os.py @@ -2624,6 +2624,40 @@ def test_execve_invalid_env(self): with self.assertRaises(ValueError): os.execve(args[0], args, newenv) + # See https://github.com/python/cpython/issues/137934 and the other + # related issues for the reason why we cannot test this on Windows. + @unittest.skipIf(os.name == "nt", "POSIX-specific test") + def test_execve_env_concurrent_mutation_with_fspath_posix(self): + # Prevent crash when mutating environment during parsing. + # Regression test for https://github.com/python/cpython/issues/143309. + + message = "hello from execve" + code = """ + import os, sys + + class MyPath: + def __fspath__(self): + mutated.clear() + return b"pwn" + + mutated = KEYS = VALUES = [MyPath()] + + class MyEnv: + def __getitem__(self): raise RuntimeError("must not be called") + def __len__(self): return 1 + def keys(self): return KEYS + def values(self): return VALUES + + args = [sys.executable, '-c', "print({message!r})"] + os.execve(args[0], args, MyEnv()) + """.format(message=message) + + # Use '__cleanenv' to signal to assert_python_ok() not + # to do a copy of os.environ on its own. + rc, out, _ = assert_python_ok('-c', code, __cleanenv=True) + self.assertEqual(rc, 0) + self.assertIn(bytes(message, "ascii"), out) + @unittest.skipUnless(sys.platform == "win32", "Win32-specific test") def test_execve_with_empty_path(self): # bpo-32890: Check GetLastError() misuse diff --git a/Misc/NEWS.d/next/Library/2025-12-31-20-43-02.gh-issue-143309.cdFxdH.rst b/Misc/NEWS.d/next/Library/2025-12-31-20-43-02.gh-issue-143309.cdFxdH.rst new file mode 100644 index 00000000000000..5f30ed340bf0eb --- /dev/null +++ b/Misc/NEWS.d/next/Library/2025-12-31-20-43-02.gh-issue-143309.cdFxdH.rst @@ -0,0 +1,3 @@ +Fix a crash in :func:`os.execve` on non-Windows platforms when +given a custom environment mapping which is then mutated during +parsing. Patch by Bénédikt Tran. diff --git a/Modules/posixmodule.c b/Modules/posixmodule.c index e0276ce9e3906f..b26785abc3cd27 100644 --- a/Modules/posixmodule.c +++ b/Modules/posixmodule.c @@ -7291,8 +7291,8 @@ static EXECV_CHAR** parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) { Py_ssize_t i, pos, envc; - PyObject *keys=NULL, *vals=NULL; - PyObject *key2, *val2, *keyval; + PyObject *keys = NULL, *vals = NULL; + PyObject *key = NULL, *val = NULL, *key2 = NULL, *val2 = NULL; EXECV_CHAR **envlist; i = PyMapping_Size(env); @@ -7317,20 +7317,22 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) } for (pos = 0; pos < i; pos++) { - PyObject *key = PyList_GetItem(keys, pos); // Borrowed ref. + // The 'key' and 'val' must be strong references because of + // possible side-effects by PyUnicode_FS{Converter,Decoder}(). + key = PyList_GetItemRef(keys, pos); if (key == NULL) { goto error; } - PyObject *val = PyList_GetItem(vals, pos); // Borrowed ref. + val = PyList_GetItemRef(vals, pos); if (val == NULL) { goto error; } #if defined(HAVE_WEXECV) || defined(HAVE_WSPAWNV) - if (!PyUnicode_FSDecoder(key, &key2)) + if (!PyUnicode_FSDecoder(key, &key2)) { goto error; + } if (!PyUnicode_FSDecoder(val, &val2)) { - Py_DECREF(key2); goto error; } /* Search from index 1 because on Windows starting '=' is allowed for @@ -7339,39 +7341,38 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) PyUnicode_FindChar(key2, '=', 1, PyUnicode_GET_LENGTH(key2), 1) != -1) { PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); - Py_DECREF(key2); - Py_DECREF(val2); goto error; } - keyval = PyUnicode_FromFormat("%U=%U", key2, val2); + PyObject *keyval = PyUnicode_FromFormat("%U=%U", key2, val2); #else - if (!PyUnicode_FSConverter(key, &key2)) + if (!PyUnicode_FSConverter(key, &key2)) { goto error; + } if (!PyUnicode_FSConverter(val, &val2)) { - Py_DECREF(key2); goto error; } if (PyBytes_GET_SIZE(key2) == 0 || strchr(PyBytes_AS_STRING(key2) + 1, '=') != NULL) { PyErr_SetString(PyExc_ValueError, "illegal environment variable name"); - Py_DECREF(key2); - Py_DECREF(val2); goto error; } - keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2), - PyBytes_AS_STRING(val2)); + PyObject *keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2), + PyBytes_AS_STRING(val2)); #endif - Py_DECREF(key2); - Py_DECREF(val2); - if (!keyval) + if (!keyval) { goto error; + } if (!fsconvert_strdup(keyval, &envlist[envc++])) { Py_DECREF(keyval); goto error; } + Py_CLEAR(key); + Py_CLEAR(val); + Py_CLEAR(key2); + Py_CLEAR(val2); Py_DECREF(keyval); } Py_DECREF(vals); @@ -7382,6 +7383,10 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr) return envlist; error: + Py_XDECREF(key); + Py_XDECREF(val); + Py_XDECREF(key2); + Py_XDECREF(val2); Py_XDECREF(keys); Py_XDECREF(vals); free_string_array(envlist, envc);