Skip to content

Commit 11a2482

Browse files
authored
[3.14] gh-151416: fix a borrowed ref potential use after free via fspath in os.spawnv/spawnve (GH-151417) (#152536)
gh-151416: fix a borrowed ref potential use after free via fspath in os.spawnv/spawnve (GH-151417) * gh-151416: Fix use-after-free in os.spawnv/spawnve when __fspath__ mutates argv The argv conversion loops passed references borrowed from the argv list into fsconvert_strdup(). An item's __fspath__() can mutate the list and release its reference to the item, leaving the converter operating on a freed object. A shrunk list could also make PyList_GetItem() return NULL, which PyUnicode_FS{Converter,Decoder}() treat as a request to release an uninitialized output variable. Hold a strong reference to each item across the conversion, matching parse_arglist() and parse_envlist(). * gh-151416: Don't mask non-TypeError argv conversion errors in os.spawnv os.spawnv() replaced any error raised during argv item conversion, such as MemoryError, codec errors, or the embedded-null ValueError, with a generic TypeError. Only add the contextual message when the conversion actually raised TypeError, matching how os.spawnve() and the exec functions propagate these errors. The test is gated to the native C spawnv: the Python fallback used elsewhere reports conversion failures from the forked child as exit status 127 instead of raising. (cherry picked from commit f57d3d6)
1 parent 84badb7 commit 11a2482

3 files changed

Lines changed: 65 additions & 14 deletions

File tree

Lib/test/test_os.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,15 @@ def create_file(filename, content=b'content'):
9898
fp.write(content)
9999

100100

101+
# On platforms without a native spawnv(), os.py provides a Python fallback
102+
# built on fork()+exec*() that reports argument conversion failures from the
103+
# child as exit status 127 instead of raising, so tests of the C
104+
# implementation's error paths cannot run against it.
105+
requires_native_spawnv = unittest.skipUnless(
106+
isinstance(getattr(os, 'spawnv', None), types.BuiltinFunctionType),
107+
'requires the native C os.spawnv')
108+
109+
101110
# bpo-41625: On AIX, splice() only works with a socket, not with a pipe.
102111
requires_splice_pipe = unittest.skipIf(sys.platform.startswith("aix"),
103112
'on AIX, splice() only accepts sockets')
@@ -3762,6 +3771,25 @@ def test_spawnve_noargs(self):
37623771
self.assertRaises(ValueError, os.spawnve, os.P_NOWAIT, program, ('',), {})
37633772
self.assertRaises(ValueError, os.spawnve, os.P_NOWAIT, program, [''], {})
37643773

3774+
@requires_native_spawnv
3775+
def test_spawnv_arg_conversion_errors(self):
3776+
# A non-path argv item gets a TypeError naming the argument...
3777+
with self.assertRaisesRegex(TypeError, 'must contain only strings'):
3778+
os.spawnv(os.P_NOWAIT, sys.executable, [sys.executable, 123])
3779+
# ...but other conversion errors must not be masked as TypeError
3780+
# (gh-151416).
3781+
with self.assertRaises(ValueError):
3782+
os.spawnv(os.P_NOWAIT, sys.executable,
3783+
[sys.executable, 'embedded\0null'])
3784+
3785+
class RaisingPath:
3786+
def __fspath__(self):
3787+
raise RuntimeError('gotcha')
3788+
3789+
with self.assertRaisesRegex(RuntimeError, 'gotcha'):
3790+
os.spawnv(os.P_NOWAIT, sys.executable,
3791+
[sys.executable, RaisingPath()])
3792+
37653793
def _test_invalid_env(self, spawn):
37663794
program = sys.executable
37673795
args = self.quote_args([program, '-c', 'pass'])
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
Fix a crash in :func:`os.spawnv` and :func:`os.spawnve` when an *argv*
2+
item's :meth:`~os.PathLike.__fspath__` method mutates the *argv* list
3+
during argument conversion. :func:`!os.spawnv` argument conversion errors
4+
other than :exc:`TypeError`, such as the :exc:`ValueError` for an embedded
5+
null, are no longer replaced with a generic :exc:`TypeError`.

Modules/posixmodule.c

Lines changed: 32 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -7729,18 +7729,15 @@ os_spawnv_impl(PyObject *module, int mode, path_t *path, PyObject *argv)
77297729
int i;
77307730
Py_ssize_t argc;
77317731
intptr_t spawnval;
7732-
PyObject *(*getitem)(PyObject *, Py_ssize_t);
77337732

77347733
/* spawnv has three arguments: (mode, path, argv), where
77357734
argv is a list or tuple of strings. */
77367735

77377736
if (PyList_Check(argv)) {
77387737
argc = PyList_Size(argv);
7739-
getitem = PyList_GetItem;
77407738
}
77417739
else if (PyTuple_Check(argv)) {
77427740
argc = PyTuple_Size(argv);
7743-
getitem = PyTuple_GetItem;
77447741
}
77457742
else {
77467743
PyErr_SetString(PyExc_TypeError,
@@ -7758,14 +7755,29 @@ os_spawnv_impl(PyObject *module, int mode, path_t *path, PyObject *argv)
77587755
return PyErr_NoMemory();
77597756
}
77607757
for (i = 0; i < argc; i++) {
7761-
if (!fsconvert_strdup((*getitem)(argv, i),
7762-
&argvlist[i])) {
7758+
// The item must be a strong reference because of possible
7759+
// side-effects of PyUnicode_FS{Converter,Decoder}() in
7760+
// fsconvert_strdup(): an item's __fspath__() can mutate a list
7761+
// *argv*, releasing the list's reference to the item (gh-151416).
7762+
PyObject *item = PySequence_ITEM(argv, i);
7763+
if (item == NULL) {
77637764
free_string_array(argvlist, i);
7764-
PyErr_SetString(
7765-
PyExc_TypeError,
7766-
"spawnv() arg 2 must contain only strings");
77677765
return NULL;
77687766
}
7767+
if (!fsconvert_strdup(item, &argvlist[i])) {
7768+
Py_DECREF(item);
7769+
free_string_array(argvlist, i);
7770+
// Add argument context to the converter's terse TypeError, but
7771+
// let MemoryError, codec errors, embedded-null ValueError, etc.
7772+
// propagate unmasked.
7773+
if (PyErr_ExceptionMatches(PyExc_TypeError)) {
7774+
PyErr_SetString(
7775+
PyExc_TypeError,
7776+
"spawnv() arg 2 must contain only strings");
7777+
}
7778+
return NULL;
7779+
}
7780+
Py_DECREF(item);
77697781
if (i == 0 && !argvlist[0][0]) {
77707782
free_string_array(argvlist, i + 1);
77717783
PyErr_SetString(
@@ -7836,7 +7848,6 @@ os_spawnve_impl(PyObject *module, int mode, path_t *path, PyObject *argv,
78367848
PyObject *res = NULL;
78377849
Py_ssize_t argc, i, envc;
78387850
intptr_t spawnval;
7839-
PyObject *(*getitem)(PyObject *, Py_ssize_t);
78407851
Py_ssize_t lastarg = 0;
78417852

78427853
/* spawnve has four arguments: (mode, path, argv, env), where
@@ -7845,11 +7856,9 @@ os_spawnve_impl(PyObject *module, int mode, path_t *path, PyObject *argv,
78457856

78467857
if (PyList_Check(argv)) {
78477858
argc = PyList_Size(argv);
7848-
getitem = PyList_GetItem;
78497859
}
78507860
else if (PyTuple_Check(argv)) {
78517861
argc = PyTuple_Size(argv);
7852-
getitem = PyTuple_GetItem;
78537862
}
78547863
else {
78557864
PyErr_SetString(PyExc_TypeError,
@@ -7873,12 +7882,21 @@ os_spawnve_impl(PyObject *module, int mode, path_t *path, PyObject *argv,
78737882
goto fail_0;
78747883
}
78757884
for (i = 0; i < argc; i++) {
7876-
if (!fsconvert_strdup((*getitem)(argv, i),
7877-
&argvlist[i]))
7878-
{
7885+
// The item must be a strong reference because of possible
7886+
// side-effects of PyUnicode_FS{Converter,Decoder}() in
7887+
// fsconvert_strdup(): an item's __fspath__() can mutate a list
7888+
// *argv*, releasing the list's reference to the item (gh-151416).
7889+
PyObject *item = PySequence_ITEM(argv, i);
7890+
if (item == NULL) {
78797891
lastarg = i;
78807892
goto fail_1;
78817893
}
7894+
if (!fsconvert_strdup(item, &argvlist[i])) {
7895+
Py_DECREF(item);
7896+
lastarg = i;
7897+
goto fail_1;
7898+
}
7899+
Py_DECREF(item);
78827900
if (i == 0 && !argvlist[0][0]) {
78837901
lastarg = i + 1;
78847902
PyErr_SetString(

0 commit comments

Comments
 (0)