From aec95c217260ec8d67ad16c1180ee52cc5a749d8 Mon Sep 17 00:00:00 2001 From: maurycy <5383+maurycy@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:03:13 +0200 Subject: [PATCH 1/2] CHECK_TUPLE_ITEMS, CHECK_LIST macros --- Modules/_remote_debugging/binary_io_writer.c | 44 ++++++++++++-------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/Modules/_remote_debugging/binary_io_writer.c b/Modules/_remote_debugging/binary_io_writer.c index 753d0b0cc966d08..0896c2a342c9dba 100644 --- a/Modules/_remote_debugging/binary_io_writer.c +++ b/Modules/_remote_debugging/binary_io_writer.c @@ -44,6 +44,22 @@ } \ } while (0) +#define CHECK_TUPLE_ITEMS(obj, n) do { \ + if (!PyTuple_Check(obj) || PyTuple_GET_SIZE(obj) < (n)) { \ + PyErr_Format(PyExc_TypeError, \ + #obj " must be a tuple of at least %zd items", \ + (Py_ssize_t)(n)); \ + return -1; \ + } \ +} while (0) + +#define CHECK_LIST(obj) do { \ + if (!PyList_Check(obj)) { \ + PyErr_SetString(PyExc_TypeError, #obj " must be a list"); \ + return -1; \ + } \ +} while (0) + /* ============================================================================ * WRITER-SPECIFIC UTILITY HELPERS * ============================================================================ */ @@ -838,8 +854,8 @@ build_frame_stack(BinaryWriter *writer, PyObject *frame_list, *curr_depth = (stack_depth < MAX_STACK_DEPTH) ? stack_depth : MAX_STACK_DEPTH; for (Py_ssize_t k = 0; k < (Py_ssize_t)*curr_depth; k++) { - /* Use unchecked accessors since we control the data structures */ PyObject *frame_info = PyList_GET_ITEM(frame_list, k); + CHECK_TUPLE_ITEMS(frame_info, 4); /* Get filename, location, funcname, opcode from FrameInfo using unchecked access */ PyObject *filename = PyStructSequence_GET_ITEM(frame_info, 0); @@ -854,20 +870,13 @@ build_frame_stack(BinaryWriter *writer, PyObject *frame_list, int32_t end_column = LOCATION_NOT_AVAILABLE; if (location != Py_None) { + CHECK_TUPLE_ITEMS(location, 4); /* LocationInfo is a struct sequence or tuple with: * (lineno, end_lineno, column, end_column) */ - PyObject *lineno_obj = PyTuple_Check(location) ? - PyTuple_GET_ITEM(location, 0) : - PyStructSequence_GET_ITEM(location, 0); - PyObject *end_lineno_obj = PyTuple_Check(location) ? - PyTuple_GET_ITEM(location, 1) : - PyStructSequence_GET_ITEM(location, 1); - PyObject *column_obj = PyTuple_Check(location) ? - PyTuple_GET_ITEM(location, 2) : - PyStructSequence_GET_ITEM(location, 2); - PyObject *end_column_obj = PyTuple_Check(location) ? - PyTuple_GET_ITEM(location, 3) : - PyStructSequence_GET_ITEM(location, 3); + PyObject *lineno_obj = PyTuple_GET_ITEM(location, 0); + PyObject *end_lineno_obj = PyTuple_GET_ITEM(location, 1); + PyObject *column_obj = PyTuple_GET_ITEM(location, 2); + PyObject *end_column_obj = PyTuple_GET_ITEM(location, 3); PYLONG_TO_INT32_OR_DEFAULT(lineno_obj, lineno, LOCATION_NOT_AVAILABLE); PYLONG_TO_INT32_OR_DEFAULT(end_lineno_obj, end_lineno, LOCATION_NOT_AVAILABLE); @@ -925,9 +934,11 @@ static int process_thread_sample(BinaryWriter *writer, PyObject *thread_info, uint32_t interpreter_id, uint64_t timestamp_us) { + CHECK_TUPLE_ITEMS(thread_info, 3); PyObject *thread_id_obj = PyStructSequence_GET_ITEM(thread_info, 0); PyObject *status_obj = PyStructSequence_GET_ITEM(thread_info, 1); PyObject *frame_list = PyStructSequence_GET_ITEM(thread_info, 2); + CHECK_LIST(frame_list); uint64_t thread_id = PyLong_AsUnsignedLongLong(thread_id_obj); if (thread_id == (uint64_t)-1 && PyErr_Occurred()) { @@ -1010,17 +1021,16 @@ process_thread_sample(BinaryWriter *writer, PyObject *thread_info, int binary_writer_write_sample(BinaryWriter *writer, PyObject *stack_frames, uint64_t timestamp_us) { - if (!PyList_Check(stack_frames)) { - PyErr_SetString(PyExc_TypeError, "stack_frames must be a list"); - return -1; - } + CHECK_LIST(stack_frames); Py_ssize_t num_interpreters = PyList_GET_SIZE(stack_frames); for (Py_ssize_t i = 0; i < num_interpreters; i++) { PyObject *interp_info = PyList_GET_ITEM(stack_frames, i); + CHECK_TUPLE_ITEMS(interp_info, 2); PyObject *interp_id_obj = PyStructSequence_GET_ITEM(interp_info, 0); PyObject *threads = PyStructSequence_GET_ITEM(interp_info, 1); + CHECK_LIST(threads); unsigned long interp_id_long = PyLong_AsUnsignedLong(interp_id_obj); if (interp_id_long == (unsigned long)-1 && PyErr_Occurred()) { From 9a75080fe278fcbab2e36c483678a0253bf6bef5 Mon Sep 17 00:00:00 2001 From: maurycy <5383+maurycy@users.noreply.github.com> Date: Wed, 9 Sep 2026 20:12:14 +0200 Subject: [PATCH 2/2] test --- .../test_binary_format.py | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py b/Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py index 253f419dae3c2e7..a3b5ab97f58f6b1 100644 --- a/Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py +++ b/Lib/test/test_profiling/test_sampling_profiler/test_binary_format.py @@ -844,6 +844,42 @@ def test_invalid_path_error_preserves_pathlib(self): _remote_debugging.BinaryReader(missing) self.assertEqual(os.fspath(cm.exception.filename), os.fspath(missing)) + def test_writer_rejects_malformed_samples(self): + """Malformed sample containers raise TypeError instead of crashing.""" + cases = ( + ("stack_frames", 42), + ("interp_info", [42]), + ("interp_info", [()]), + ("interp_info", [(0,)]), + ("threads", [(0, 42)]), + ("thread_info", [(0, [42])]), + ("thread_info", [(0, [()])]), + ("thread_info", [(0, [(1,)])]), + ("thread_info", [(0, [(1, 0)])]), + ("frame_list", [(0, [(1, 0, 42)])]), + ("frame_info", [(0, [(1, 0, [42])])]), + ("frame_info", [(0, [(1, 0, [()])])]), + ("frame_info", [(0, [(1, 0, [("a.py",)])])]), + ("frame_info", [(0, [(1, 0, [("a.py", None)])])]), + ("frame_info", [(0, [(1, 0, [("a.py", None, "f")])])]), + ("location", [(0, [(1, 0, [("a.py", 42, "f", None)])])]), + ("location", [(0, [(1, 0, [("a.py", (), "f", None)])])]), + ("location", [(0, [(1, 0, [("a.py", (1,), "f", None)])])]), + ("location", [(0, [(1, 0, [("a.py", (1, 1), "f", None)])])]), + ("location", [(0, [(1, 0, [("a.py", (1, 1, 0), "f", None)])])]), + ) + with tempfile.NamedTemporaryFile(suffix=".bin", delete=False) as f: + filename = f.name + self.temp_files.append(filename) + + for field, sample in cases: + with self.subTest(field=field, sample=sample): + with _remote_debugging.BinaryWriter( + filename, 1000, 0, compression=0 + ) as writer: + with self.assertRaisesRegex(TypeError, field): + writer.write_sample(sample, 2000) + def test_writer_handles_empty_stack_first_sample(self): """BinaryWriter.write_sample tolerates an empty stack on a fresh thread.