Skip to content

Commit 243b880

Browse files
committed
gh-143309: fix UAF in os.execve when the environment is concurrently mutated
1 parent 3c4429f commit 243b880

File tree

3 files changed

+56
-6
lines changed

3 files changed

+56
-6
lines changed

Lib/test/test_os/test_os.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2624,6 +2624,34 @@ def test_execve_invalid_env(self):
26242624
with self.assertRaises(ValueError):
26252625
os.execve(args[0], args, newenv)
26262626

2627+
def test_execve_env_concurrent_mutation_with_fspath(self):
2628+
# Prevent crash when mutating environment during parsing.
2629+
# Regression test for https://github.com/python/cpython/issues/143309.
2630+
2631+
code = """
2632+
import os, sys
2633+
2634+
class MyPath:
2635+
def __fspath__(self):
2636+
mutated.clear()
2637+
return b"pwn"
2638+
2639+
mutated = KEYS = VALUES = [MyPath()]
2640+
2641+
class MyEnv:
2642+
def __len__(self): return 1
2643+
def __getitem__(self): return 1
2644+
def keys(self): return KEYS
2645+
def values(self): return VALUES
2646+
2647+
args = [sys.executable, '-c', 'print("hello from execve")']
2648+
os.execve(args[0], args, MyEnv())
2649+
"""
2650+
2651+
rc, out, _ = assert_python_ok('-c', code)
2652+
self.assertEqual(rc, 0)
2653+
self.assertIn(b"hello from execve", out)
2654+
26272655
@unittest.skipUnless(sys.platform == "win32", "Win32-specific test")
26282656
def test_execve_with_empty_path(self):
26292657
# bpo-32890: Check GetLastError() misuse
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix a crash in :func:`os.execve` when given a custom environment mapping
2+
which is then mutated during parsing. Patch by Bénédikt Tran.

Modules/posixmodule.c

Lines changed: 26 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7291,8 +7291,8 @@ static EXECV_CHAR**
72917291
parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
72927292
{
72937293
Py_ssize_t i, pos, envc;
7294-
PyObject *keys=NULL, *vals=NULL;
7295-
PyObject *key2, *val2, *keyval;
7294+
PyObject *keys = NULL, *vals = NULL;
7295+
PyObject *key, *val, *key2, *val2, *keyval;
72967296
EXECV_CHAR **envlist;
72977297

72987298
i = PyMapping_Size(env);
@@ -7317,19 +7317,28 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
73177317
}
73187318

73197319
for (pos = 0; pos < i; pos++) {
7320-
PyObject *key = PyList_GetItem(keys, pos); // Borrowed ref.
7320+
// The 'key' and 'val' must be strong references because
7321+
// of possible side-effects by PyUnicode_FSConverter().
7322+
key = PyList_GetItemRef(keys, pos);
73217323
if (key == NULL) {
73227324
goto error;
73237325
}
7324-
PyObject *val = PyList_GetItem(vals, pos); // Borrowed ref.
7326+
val = PyList_GetItemRef(vals, pos);
73257327
if (val == NULL) {
7328+
Py_DECREF(key);
73267329
goto error;
73277330
}
73287331

7332+
73297333
#if defined(HAVE_WEXECV) || defined(HAVE_WSPAWNV)
7330-
if (!PyUnicode_FSDecoder(key, &key2))
7334+
if (!PyUnicode_FSDecoder(key, &key2)) {
7335+
Py_DECREF(key);
7336+
Py_DECREF(val);
73317337
goto error;
7338+
}
73327339
if (!PyUnicode_FSDecoder(val, &val2)) {
7340+
Py_DECREF(key);
7341+
Py_DECREF(val);
73337342
Py_DECREF(key2);
73347343
goto error;
73357344
}
@@ -7339,29 +7348,40 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
73397348
PyUnicode_FindChar(key2, '=', 1, PyUnicode_GET_LENGTH(key2), 1) != -1)
73407349
{
73417350
PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
7351+
Py_DECREF(key);
7352+
Py_DECREF(val);
73427353
Py_DECREF(key2);
73437354
Py_DECREF(val2);
73447355
goto error;
73457356
}
73467357
keyval = PyUnicode_FromFormat("%U=%U", key2, val2);
73477358
#else
7348-
if (!PyUnicode_FSConverter(key, &key2))
7359+
if (!PyUnicode_FSConverter(key, &key2)) {
7360+
Py_DECREF(key);
7361+
Py_DECREF(val);
73497362
goto error;
7363+
}
73507364
if (!PyUnicode_FSConverter(val, &val2)) {
7365+
Py_DECREF(key);
7366+
Py_DECREF(val);
73517367
Py_DECREF(key2);
73527368
goto error;
73537369
}
73547370
if (PyBytes_GET_SIZE(key2) == 0 ||
73557371
strchr(PyBytes_AS_STRING(key2) + 1, '=') != NULL)
73567372
{
73577373
PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
7374+
Py_DECREF(key);
7375+
Py_DECREF(val);
73587376
Py_DECREF(key2);
73597377
Py_DECREF(val2);
73607378
goto error;
73617379
}
73627380
keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2),
73637381
PyBytes_AS_STRING(val2));
73647382
#endif
7383+
Py_DECREF(key);
7384+
Py_DECREF(val);
73657385
Py_DECREF(key2);
73667386
Py_DECREF(val2);
73677387
if (!keyval)

0 commit comments

Comments
 (0)