Skip to content

Commit 1374fde

Browse files
StanFromIrelandvstinnermaurycy
authored
gh-159398: Validate strings read from the remote process in _remote_debugging (#159402)
Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com>
1 parent d3a1e72 commit 1374fde

4 files changed

Lines changed: 99 additions & 26 deletions

File tree

‎Lib/test/test_external_inspection.py‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -901,6 +901,35 @@ def {func_name}():
901901
)
902902
self.assertEqual(target.filename, script_name)
903903

904+
@skip_if_not_supported
905+
@unittest.skipIf(
906+
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
907+
"Test only runs on Linux with process_vm_readv support",
908+
)
909+
def test_remote_stack_trace_invalid_unicode_name(self):
910+
import_module("_testinternalcapi")
911+
912+
script = textwrap.dedent("""\
913+
import os
914+
import _remote_debugging
915+
import _testinternalcapi
916+
917+
def func():
918+
return _remote_debugging.RemoteUnwinder(os.getpid()).get_stack_trace()
919+
920+
name = _testinternalcapi.new_invalid_str()
921+
func.__code__ = func.__code__.replace(co_qualname=name)
922+
print(func())
923+
""")
924+
result = subprocess.run(
925+
[sys.executable, "-c", script],
926+
capture_output=True,
927+
text=True,
928+
timeout=SHORT_TIMEOUT,
929+
)
930+
self.assertEqual(result.returncode, 0, result)
931+
self.assertIn("<unknown function>", result.stdout)
932+
904933
@skip_if_not_supported
905934
@unittest.skipIf(
906935
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
@@ -4575,6 +4604,19 @@ def test_long_qualname_truncated_not_dropped(self):
45754604
self.assertIsNotNone(frame)
45764605
self.assertEqual(frame.funcname, "f" * 1024)
45774606

4607+
def test_long_qualname_truncated_to_narrower_kind(self):
4608+
name = "a" * 1024 + "\U00010300"
4609+
src = f"def {name}(sample):\n return sample()\n"
4610+
ns = {}
4611+
exec(src, ns)
4612+
4613+
trace = ns[name](RemoteUnwinder(os.getpid()).get_stack_trace)
4614+
frame = self._find_frame_in_trace(
4615+
trace, lambda f: f.funcname.startswith("aaa")
4616+
)
4617+
self.assertIsNotNone(frame)
4618+
self.assertEqual(frame.funcname, "a" * 1024)
4619+
45784620
def test_long_filename_truncated(self):
45794621
"""A filename longer than 1024 chars is truncated instead of
45804622
failing the whole sample."""
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
Fix the sampling profiler crashing when a string read from the target process
2+
contains code points above ``U+10FFFF``.

‎Modules/_remote_debugging/object_reading.c‎

Lines changed: 41 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
******************************************************************************/
77

88
#include "_remote_debugging.h"
9+
#include "internal/pycore_unicodeobject.h" // _Py_MAX_UNICODE
910
#include <limits.h>
1011

1112
/* ============================================================================
@@ -91,23 +92,15 @@ read_py_str(
9192
}
9293

9394
int kind = (int)state.kind;
94-
Py_UCS4 max_char;
95-
switch (kind) {
96-
case PyUnicode_1BYTE_KIND:
97-
max_char = state.ascii ? 0x7F : 0xFF;
98-
break;
99-
case PyUnicode_2BYTE_KIND:
100-
max_char = 0xFFFF;
101-
break;
102-
case PyUnicode_4BYTE_KIND:
103-
max_char = 0x10FFFF;
104-
break;
105-
default:
106-
PyErr_Format(PyExc_RuntimeError,
107-
"Invalid Unicode kind %d at 0x%lx", kind, address);
108-
set_exception_cause(unwinder, PyExc_RuntimeError,
109-
"Invalid kind in remote Unicode object");
110-
return NULL;
95+
if (kind != PyUnicode_1BYTE_KIND
96+
&& kind != PyUnicode_2BYTE_KIND
97+
&& kind != PyUnicode_4BYTE_KIND)
98+
{
99+
PyErr_Format(PyExc_RuntimeError,
100+
"Invalid Unicode kind %d at 0x%lx", kind, address);
101+
set_exception_cause(unwinder, PyExc_RuntimeError,
102+
"Invalid kind in remote Unicode object");
103+
return NULL;
111104
}
112105

113106
size_t header_size = state.ascii
@@ -127,16 +120,16 @@ read_py_str(
127120
return NULL;
128121
}
129122

130-
PyObject *result = PyUnicode_New(len, max_char);
131-
if (result == NULL) {
132-
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to allocate PyUnicode for remote string");
133-
return NULL;
134-
}
135123
if (nbytes == 0) {
136-
return result;
124+
return Py_GetConstant(Py_CONSTANT_EMPTY_STR);
137125
}
138126

139-
void *data = PyUnicode_DATA(result);
127+
char *data = PyMem_Malloc(nbytes);
128+
if (data == NULL) {
129+
PyErr_NoMemory();
130+
set_exception_cause(unwinder, PyExc_MemoryError, "Failed to allocate buffer for remote string");
131+
return NULL;
132+
}
140133

141134
// Reuse data already present in the header read; only round-trip for
142135
// whatever spills past it.
@@ -153,14 +146,36 @@ read_py_str(
153146
&unwinder->handle,
154147
address + header_size + inline_bytes,
155148
nbytes - inline_bytes,
156-
(char *)data + inline_bytes);
149+
data + inline_bytes);
157150
if (res < 0) {
158-
Py_DECREF(result);
151+
PyMem_Free(data);
159152
set_exception_cause(unwinder, PyExc_RuntimeError, "Failed to read string data from remote memory");
160153
return NULL;
161154
}
162155
}
163156

157+
if (kind == PyUnicode_4BYTE_KIND) {
158+
Py_UCS4 *ucs4 = (Py_UCS4 *)data;
159+
for (Py_ssize_t i = 0; i < len; i++) {
160+
Py_UCS4 ch = ucs4[i];
161+
if (ch > _Py_MAX_UNICODE) {
162+
PyMem_Free(data);
163+
PyErr_Format(PyExc_RuntimeError,
164+
"character U+%x is not in range [U+0000; U+%x]",
165+
ch, _Py_MAX_UNICODE);
166+
set_exception_cause(unwinder, PyExc_RuntimeError,
167+
"Unicode string contains invalid character");
168+
return NULL;
169+
}
170+
}
171+
}
172+
173+
PyObject *result = PyUnicode_FromKindAndData(kind, data, len);
174+
PyMem_Free(data);
175+
if (result == NULL) {
176+
set_exception_cause(unwinder, PyExc_RuntimeError, "Invalid character data in remote Unicode object");
177+
return NULL;
178+
}
164179
return result;
165180
}
166181

‎Modules/_testinternalcapi.c‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2060,6 +2060,19 @@ unicode_transformdecimalandspacetoascii(PyObject *self, PyObject *arg)
20602060
return _PyUnicode_TransformDecimalAndSpaceToASCII(arg);
20612061
}
20622062

2063+
/* Return a str holding code points above U+10FFFF. */
2064+
static PyObject *
2065+
new_invalid_str(PyObject *self, PyObject *Py_UNUSED(ignored))
2066+
{
2067+
PyObject *str = PyUnicode_New(2, _Py_MAX_UNICODE);
2068+
if (str == NULL) {
2069+
return NULL;
2070+
}
2071+
PyUnicode_4BYTE_DATA(str)[0] = _Py_MAX_UNICODE + 1;
2072+
PyUnicode_4BYTE_DATA(str)[1] = _Py_MAX_UNICODE + 2;
2073+
return str;
2074+
}
2075+
20632076
static PyObject *
20642077
test_pyobject_is_freed(const char *test_name, PyObject *op)
20652078
{
@@ -3402,6 +3415,7 @@ static PyMethodDef module_functions[] = {
34023415
{"_PyTraceMalloc_GetTraceback", tracemalloc_get_traceback, METH_VARARGS},
34033416
{"test_tstate_capi", test_tstate_capi, METH_NOARGS, NULL},
34043417
{"_PyUnicode_TransformDecimalAndSpaceToASCII", unicode_transformdecimalandspacetoascii, METH_O},
3418+
{"new_invalid_str", new_invalid_str, METH_NOARGS},
34053419
{"check_pyobject_forbidden_bytes_is_freed",
34063420
check_pyobject_forbidden_bytes_is_freed, METH_NOARGS},
34073421
{"check_pyobject_freed_is_freed", check_pyobject_freed_is_freed, METH_NOARGS},

0 commit comments

Comments
 (0)