Skip to content

Commit bb85605

Browse files
miss-islingtonStanFromIrelandvstinnermaurycy
authored
[3.15] gh-159398: Validate strings read from the remote process in _remote_debugging (GH-159402) (#160963)
(cherry picked from commit 1374fde) Co-authored-by: Stan Ulbrych <stan@python.org> Co-authored-by: Victor Stinner <vstinner@python.org> Co-authored-by: Maurycy Pawłowski-Wieroński <maurycy@maurycy.com>
1 parent 8ea98fc commit bb85605

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
@@ -852,6 +852,35 @@ def {func_name}():
852852
)
853853
self.assertEqual(target.filename, script_name)
854854

855+
@skip_if_not_supported
856+
@unittest.skipIf(
857+
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
858+
"Test only runs on Linux with process_vm_readv support",
859+
)
860+
def test_remote_stack_trace_invalid_unicode_name(self):
861+
import_module("_testinternalcapi")
862+
863+
script = textwrap.dedent("""\
864+
import os
865+
import _remote_debugging
866+
import _testinternalcapi
867+
868+
def func():
869+
return _remote_debugging.RemoteUnwinder(os.getpid()).get_stack_trace()
870+
871+
name = _testinternalcapi.new_invalid_str()
872+
func.__code__ = func.__code__.replace(co_qualname=name)
873+
print(func())
874+
""")
875+
result = subprocess.run(
876+
[sys.executable, "-c", script],
877+
capture_output=True,
878+
text=True,
879+
timeout=SHORT_TIMEOUT,
880+
)
881+
self.assertEqual(result.returncode, 0, result)
882+
self.assertIn("<unknown function>", result.stdout)
883+
855884
@skip_if_not_supported
856885
@unittest.skipIf(
857886
sys.platform == "linux" and not PROCESS_VM_READV_SUPPORTED,
@@ -4452,6 +4481,19 @@ def test_long_qualname_truncated_not_dropped(self):
44524481
self.assertIsNotNone(frame)
44534482
self.assertEqual(frame.funcname, "f" * 1024)
44544483

4484+
def test_long_qualname_truncated_to_narrower_kind(self):
4485+
name = "a" * 1024 + "\U00010300"
4486+
src = f"def {name}(sample):\n return sample()\n"
4487+
ns = {}
4488+
exec(src, ns)
4489+
4490+
trace = ns[name](RemoteUnwinder(os.getpid()).get_stack_trace)
4491+
frame = self._find_frame_in_trace(
4492+
trace, lambda f: f.funcname.startswith("aaa")
4493+
)
4494+
self.assertIsNotNone(frame)
4495+
self.assertEqual(frame.funcname, "a" * 1024)
4496+
44554497
def test_long_filename_truncated(self):
44564498
"""A filename longer than 1024 chars is truncated instead of
44574499
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
@@ -2017,6 +2017,19 @@ unicode_transformdecimalandspacetoascii(PyObject *self, PyObject *arg)
20172017
return _PyUnicode_TransformDecimalAndSpaceToASCII(arg);
20182018
}
20192019

2020+
/* Return a str holding code points above U+10FFFF. */
2021+
static PyObject *
2022+
new_invalid_str(PyObject *self, PyObject *Py_UNUSED(ignored))
2023+
{
2024+
PyObject *str = PyUnicode_New(2, _Py_MAX_UNICODE);
2025+
if (str == NULL) {
2026+
return NULL;
2027+
}
2028+
PyUnicode_4BYTE_DATA(str)[0] = _Py_MAX_UNICODE + 1;
2029+
PyUnicode_4BYTE_DATA(str)[1] = _Py_MAX_UNICODE + 2;
2030+
return str;
2031+
}
2032+
20202033
static PyObject *
20212034
test_pyobject_is_freed(const char *test_name, PyObject *op)
20222035
{
@@ -3338,6 +3351,7 @@ static PyMethodDef module_functions[] = {
33383351
{"_PyTraceMalloc_GetTraceback", tracemalloc_get_traceback, METH_VARARGS},
33393352
{"test_tstate_capi", test_tstate_capi, METH_NOARGS, NULL},
33403353
{"_PyUnicode_TransformDecimalAndSpaceToASCII", unicode_transformdecimalandspacetoascii, METH_O},
3354+
{"new_invalid_str", new_invalid_str, METH_NOARGS},
33413355
{"check_pyobject_forbidden_bytes_is_freed",
33423356
check_pyobject_forbidden_bytes_is_freed, METH_NOARGS},
33433357
{"check_pyobject_freed_is_freed", check_pyobject_freed_is_freed, METH_NOARGS},

0 commit comments

Comments
 (0)