Skip to content

Commit 623dbaf

Browse files
[3.14] gh-143309: fix UAF in os.execve when the environment is concurrently mutated (GH-143314) (#143398)
gh-143309: fix UAF in `os.execve` when the environment is concurrently mutated (GH-143314) (cherry picked from commit 9609574) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com>
1 parent c99d87d commit 623dbaf

File tree

3 files changed

+60
-18
lines changed

3 files changed

+60
-18
lines changed

Lib/test/test_os.py

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2417,6 +2417,40 @@ def test_execve_invalid_env(self):
24172417
with self.assertRaises(ValueError):
24182418
os.execve(args[0], args, newenv)
24192419

2420+
# See https://github.com/python/cpython/issues/137934 and the other
2421+
# related issues for the reason why we cannot test this on Windows.
2422+
@unittest.skipIf(os.name == "nt", "POSIX-specific test")
2423+
def test_execve_env_concurrent_mutation_with_fspath_posix(self):
2424+
# Prevent crash when mutating environment during parsing.
2425+
# Regression test for https://github.com/python/cpython/issues/143309.
2426+
2427+
message = "hello from execve"
2428+
code = """
2429+
import os, sys
2430+
2431+
class MyPath:
2432+
def __fspath__(self):
2433+
mutated.clear()
2434+
return b"pwn"
2435+
2436+
mutated = KEYS = VALUES = [MyPath()]
2437+
2438+
class MyEnv:
2439+
def __getitem__(self): raise RuntimeError("must not be called")
2440+
def __len__(self): return 1
2441+
def keys(self): return KEYS
2442+
def values(self): return VALUES
2443+
2444+
args = [sys.executable, '-c', "print({message!r})"]
2445+
os.execve(args[0], args, MyEnv())
2446+
""".format(message=message)
2447+
2448+
# Use '__cleanenv' to signal to assert_python_ok() not
2449+
# to do a copy of os.environ on its own.
2450+
rc, out, _ = assert_python_ok('-c', code, __cleanenv=True)
2451+
self.assertEqual(rc, 0)
2452+
self.assertIn(bytes(message, "ascii"), out)
2453+
24202454
@unittest.skipUnless(sys.platform == "win32", "Win32-specific test")
24212455
def test_execve_with_empty_path(self):
24222456
# bpo-32890: Check GetLastError() misuse
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
Fix a crash in :func:`os.execve` on non-Windows platforms when
2+
given a custom environment mapping which is then mutated during
3+
parsing. Patch by Bénédikt Tran.

Modules/posixmodule.c

Lines changed: 23 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -6835,8 +6835,8 @@ static EXECV_CHAR**
68356835
parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
68366836
{
68376837
Py_ssize_t i, pos, envc;
6838-
PyObject *keys=NULL, *vals=NULL;
6839-
PyObject *key2, *val2, *keyval;
6838+
PyObject *keys = NULL, *vals = NULL;
6839+
PyObject *key = NULL, *val = NULL, *key2 = NULL, *val2 = NULL;
68406840
EXECV_CHAR **envlist;
68416841

68426842
i = PyMapping_Size(env);
@@ -6861,20 +6861,22 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
68616861
}
68626862

68636863
for (pos = 0; pos < i; pos++) {
6864-
PyObject *key = PyList_GetItem(keys, pos); // Borrowed ref.
6864+
// The 'key' and 'val' must be strong references because of
6865+
// possible side-effects by PyUnicode_FS{Converter,Decoder}().
6866+
key = PyList_GetItemRef(keys, pos);
68656867
if (key == NULL) {
68666868
goto error;
68676869
}
6868-
PyObject *val = PyList_GetItem(vals, pos); // Borrowed ref.
6870+
val = PyList_GetItemRef(vals, pos);
68696871
if (val == NULL) {
68706872
goto error;
68716873
}
68726874

68736875
#if defined(HAVE_WEXECV) || defined(HAVE_WSPAWNV)
6874-
if (!PyUnicode_FSDecoder(key, &key2))
6876+
if (!PyUnicode_FSDecoder(key, &key2)) {
68756877
goto error;
6878+
}
68766879
if (!PyUnicode_FSDecoder(val, &val2)) {
6877-
Py_DECREF(key2);
68786880
goto error;
68796881
}
68806882
/* Search from index 1 because on Windows starting '=' is allowed for
@@ -6883,39 +6885,38 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
68836885
PyUnicode_FindChar(key2, '=', 1, PyUnicode_GET_LENGTH(key2), 1) != -1)
68846886
{
68856887
PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
6886-
Py_DECREF(key2);
6887-
Py_DECREF(val2);
68886888
goto error;
68896889
}
6890-
keyval = PyUnicode_FromFormat("%U=%U", key2, val2);
6890+
PyObject *keyval = PyUnicode_FromFormat("%U=%U", key2, val2);
68916891
#else
6892-
if (!PyUnicode_FSConverter(key, &key2))
6892+
if (!PyUnicode_FSConverter(key, &key2)) {
68936893
goto error;
6894+
}
68946895
if (!PyUnicode_FSConverter(val, &val2)) {
6895-
Py_DECREF(key2);
68966896
goto error;
68976897
}
68986898
if (PyBytes_GET_SIZE(key2) == 0 ||
68996899
strchr(PyBytes_AS_STRING(key2) + 1, '=') != NULL)
69006900
{
69016901
PyErr_SetString(PyExc_ValueError, "illegal environment variable name");
6902-
Py_DECREF(key2);
6903-
Py_DECREF(val2);
69046902
goto error;
69056903
}
6906-
keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2),
6907-
PyBytes_AS_STRING(val2));
6904+
PyObject *keyval = PyBytes_FromFormat("%s=%s", PyBytes_AS_STRING(key2),
6905+
PyBytes_AS_STRING(val2));
69086906
#endif
6909-
Py_DECREF(key2);
6910-
Py_DECREF(val2);
6911-
if (!keyval)
6907+
if (!keyval) {
69126908
goto error;
6909+
}
69136910

69146911
if (!fsconvert_strdup(keyval, &envlist[envc++])) {
69156912
Py_DECREF(keyval);
69166913
goto error;
69176914
}
69186915

6916+
Py_CLEAR(key);
6917+
Py_CLEAR(val);
6918+
Py_CLEAR(key2);
6919+
Py_CLEAR(val2);
69196920
Py_DECREF(keyval);
69206921
}
69216922
Py_DECREF(vals);
@@ -6926,6 +6927,10 @@ parse_envlist(PyObject* env, Py_ssize_t *envc_ptr)
69266927
return envlist;
69276928

69286929
error:
6930+
Py_XDECREF(key);
6931+
Py_XDECREF(val);
6932+
Py_XDECREF(key2);
6933+
Py_XDECREF(val2);
69296934
Py_XDECREF(keys);
69306935
Py_XDECREF(vals);
69316936
free_string_array(envlist, envc);

0 commit comments

Comments
 (0)