mirror of
https://github.com/python/cpython.git
synced 2026-01-06 23:42:34 +00:00
gh-143309: fix UAF in os.execve when the environment is concurrently mutated (#143314)
This commit is contained in:
parent
6c53af18f6
commit
9609574e7f
3 changed files with 60 additions and 18 deletions
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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.
|
||||
|
|
@ -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);
|
||||
|
|
|
|||
Loading…
Add table
Add a link
Reference in a new issue