From a1c26039dce3faf7a73db97ac15d060cdc5666c2 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 3 Jan 2026 10:23:37 +0300 Subject: [PATCH 01/10] gh-143379: fix UAF in struct.Struct.pack() when object modified by dunder methods --- Lib/test/test_struct.py | 11 ++++++ ...-01-03-10-15-32.gh-issue-143379.iz-hU7.rst | 4 +++ Modules/_struct.c | 34 ++++++++++++++----- 3 files changed, 41 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index cceecdd526c006..cc7201e5431265 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -817,6 +817,17 @@ def test_endian_table_init_subinterpreters(self): results = executor.map(exec, [code] * 5) self.assertListEqual(list(results), [None] * 5) + def test_Struct_object_mutation_via_dunders(self): + S = struct.Struct('?I') + + class Evil(): + def __bool__(self): + # This rebuilds format codes during S.pack(). + S.__init__('I') + return True + + self.assertEqual(S.pack(Evil(), 1), struct.Struct('?I').pack(True, 1)) + class UnpackIteratorTest(unittest.TestCase): """ diff --git a/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst b/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst new file mode 100644 index 00000000000000..17eed480ebf4cc --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst @@ -0,0 +1,4 @@ +Fix use-after-free in :meth:`struct.Struct.pack` when the +:class:`struct.Struct` object is mutated by dunder methods (like +:meth:`object.__bool__`) during packing of arguments. Patch by Sergey B +Kirpichev. diff --git a/Modules/_struct.c b/Modules/_struct.c index 2acb3df3a30395..ba55de453f62ea 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -2139,21 +2139,31 @@ Struct_iter_unpack_impl(PyStructObject *self, PyObject *buffer) * argument for where to start processing the arguments for packing, and a * character buffer for writing the packed string. The caller must insure * that the buffer may contain the required length for packing the arguments. - * 0 is returned on success, 1 is returned if there is an error. + * 0 is returned on success, -1 is returned if there is an error. * */ static int s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, char* buf, _structmodulestate *state) { - formatcode *code; + formatcode *code, *frozen_codes; /* XXX(nnorwitz): why does i need to be a local? can we use the offset parameter or do we need the wider width? */ - Py_ssize_t i; + Py_ssize_t i, frozen_size = 1; + /* Take copy of soself->s_codes, as dunder methods of arguments (e.g. + __bool__ of __float__) could modify the struct object during packing. */ + for (code = soself->s_codes; code->fmtdef != NULL; code++) { + frozen_size++; + } + frozen_codes = PyMem_Malloc(frozen_size * sizeof(formatcode)); + if (!frozen_codes) { + goto err; + } + memcpy(frozen_codes, soself->s_codes, frozen_size * sizeof(formatcode)); memset(buf, '\0', soself->s_size); i = offset; - for (code = soself->s_codes; code->fmtdef != NULL; code++) { + for (code = frozen_codes; code->fmtdef != NULL; code++) { const formatdef *e = code->fmtdef; char *res = buf + code->offset; Py_ssize_t j = code->repeat; @@ -2167,7 +2177,7 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, if (!isstring && !PyByteArray_Check(v)) { PyErr_SetString(state->StructError, "argument for 's' must be a bytes object"); - return -1; + goto err; } if (isstring) { n = PyBytes_GET_SIZE(v); @@ -2189,7 +2199,7 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, if (!isstring && !PyByteArray_Check(v)) { PyErr_SetString(state->StructError, "argument for 'p' must be a bytes object"); - return -1; + goto err; } if (isstring) { n = PyBytes_GET_SIZE(v); @@ -2215,7 +2225,7 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, if (PyLong_Check(v) && PyErr_ExceptionMatches(PyExc_OverflowError)) PyErr_SetString(state->StructError, "int too large to convert"); - return -1; + goto err; } } res += code->size; @@ -2223,7 +2233,12 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, } /* Success */ + PyMem_Free(frozen_codes); return 0; +err: + /* Failure */ + PyMem_Free(frozen_codes); + return -1; } @@ -2257,6 +2272,9 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs) return NULL; } char *buf = PyBytesWriter_GetData(writer); + /* Take copy of soself->s_size, as dunder methods of arguments (e.g. + __bool__ of __float__) could modify the struct object during packing. */ + Py_ssize_t frozen_size = soself->s_size; /* Call the guts */ if ( s_pack_internal(soself, args, 0, buf, state) != 0 ) { @@ -2264,7 +2282,7 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs) return NULL; } - return PyBytesWriter_FinishWithSize(writer, soself->s_size); + return PyBytesWriter_FinishWithSize(writer, frozen_size); } PyDoc_STRVAR(s_pack_into__doc__, From e611953812a84c985ee25bbd8b4d871430f60462 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 10 Jan 2026 05:08:48 +0300 Subject: [PATCH 02/10] address review: implement a different solution with a mutex flag Now modification of the Struct() while packing trigger a RuntimeError --- Lib/test/test_struct.py | 6 ++- ...-01-03-10-15-32.gh-issue-143379.iz-hU7.rst | 4 +- Modules/_struct.c | 51 +++++++++---------- 3 files changed, 31 insertions(+), 30 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index c2a9792dc2cb82..8811388f4ea143 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -579,7 +579,7 @@ def test_Struct_reinitialization(self): def check_sizeof(self, format_str, number_of_codes): # The size of 'PyStructObject' - totalsize = support.calcobjsize('2n3P') + totalsize = support.calcobjsize('2n3PB') # The size taken up by the 'formatcode' dynamic array totalsize += struct.calcsize('P3n0P') * (number_of_codes + 1) support.check_sizeof(self, struct.Struct(format_str), totalsize) @@ -818,6 +818,7 @@ def test_endian_table_init_subinterpreters(self): def test_Struct_object_mutation_via_dunders(self): S = struct.Struct('?I') + buf = array.array('b', b' '*100) class Evil(): def __bool__(self): @@ -825,7 +826,8 @@ def __bool__(self): S.__init__('I') return True - self.assertEqual(S.pack(Evil(), 1), struct.Struct('?I').pack(True, 1)) + self.assertRaises(RuntimeError, S.pack, Evil(), 1) + self.assertRaises(RuntimeError, S.pack_into, buf, 0, Evil(), 1) class UnpackIteratorTest(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst b/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst index 17eed480ebf4cc..c2612ef27a558c 100644 --- a/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst +++ b/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst @@ -1,4 +1,4 @@ Fix use-after-free in :meth:`struct.Struct.pack` when the :class:`struct.Struct` object is mutated by dunder methods (like -:meth:`object.__bool__`) during packing of arguments. Patch by Sergey B -Kirpichev. +:meth:`object.__bool__`) during packing of arguments. Now this trigger +:exc:`RuntimeError`. Patch by Sergey B Kirpichev. diff --git a/Modules/_struct.c b/Modules/_struct.c index ba55de453f62ea..d97f6d4a85835f 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -70,6 +70,7 @@ typedef struct { formatcode *s_codes; PyObject *s_format; PyObject *weakreflist; /* List of weak references */ + PyMutex mutex; /* to prevent mutation during packing */ } PyStructObject; #define PyStructObject_CAST(op) ((PyStructObject *)(op)) @@ -1773,6 +1774,7 @@ s_new(PyTypeObject *type, PyObject *args, PyObject *kwds) s->s_codes = NULL; s->s_size = -1; s->s_len = -1; + s->mutex = (PyMutex){0}; } return self; } @@ -1816,6 +1818,11 @@ Struct___init___impl(PyStructObject *self, PyObject *format) Py_SETREF(self->s_format, format); + if (PyMutex_IsLocked(&self->mutex)) { + PyErr_SetString(PyExc_RuntimeError, + "Call Struct.__init__() in struct.pack()"); + return -1; + } ret = prepare_s(self); return ret; } @@ -2146,24 +2153,14 @@ static int s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, char* buf, _structmodulestate *state) { - formatcode *code, *frozen_codes; + formatcode *code; /* XXX(nnorwitz): why does i need to be a local? can we use the offset parameter or do we need the wider width? */ - Py_ssize_t i, frozen_size = 1; + Py_ssize_t i; - /* Take copy of soself->s_codes, as dunder methods of arguments (e.g. - __bool__ of __float__) could modify the struct object during packing. */ - for (code = soself->s_codes; code->fmtdef != NULL; code++) { - frozen_size++; - } - frozen_codes = PyMem_Malloc(frozen_size * sizeof(formatcode)); - if (!frozen_codes) { - goto err; - } - memcpy(frozen_codes, soself->s_codes, frozen_size * sizeof(formatcode)); memset(buf, '\0', soself->s_size); i = offset; - for (code = frozen_codes; code->fmtdef != NULL; code++) { + for (code = soself->s_codes; code->fmtdef != NULL; code++) { const formatdef *e = code->fmtdef; char *res = buf + code->offset; Py_ssize_t j = code->repeat; @@ -2177,7 +2174,7 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, if (!isstring && !PyByteArray_Check(v)) { PyErr_SetString(state->StructError, "argument for 's' must be a bytes object"); - goto err; + return -1; } if (isstring) { n = PyBytes_GET_SIZE(v); @@ -2199,7 +2196,7 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, if (!isstring && !PyByteArray_Check(v)) { PyErr_SetString(state->StructError, "argument for 'p' must be a bytes object"); - goto err; + return -1; } if (isstring) { n = PyBytes_GET_SIZE(v); @@ -2225,7 +2222,7 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, if (PyLong_Check(v) && PyErr_ExceptionMatches(PyExc_OverflowError)) PyErr_SetString(state->StructError, "int too large to convert"); - goto err; + return -1; } } res += code->size; @@ -2233,12 +2230,7 @@ s_pack_internal(PyStructObject *soself, PyObject *const *args, int offset, } /* Success */ - PyMem_Free(frozen_codes); return 0; -err: - /* Failure */ - PyMem_Free(frozen_codes); - return -1; } @@ -2272,17 +2264,22 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs) return NULL; } char *buf = PyBytesWriter_GetData(writer); - /* Take copy of soself->s_size, as dunder methods of arguments (e.g. - __bool__ of __float__) could modify the struct object during packing. */ - Py_ssize_t frozen_size = soself->s_size; /* Call the guts */ + if (PyMutex_IsLocked(&soself->mutex)) { + PyErr_SetString(PyExc_RuntimeError, "XXX"); + PyBytesWriter_Discard(writer); + return NULL; + } + PyMutex_Lock(&soself->mutex); if ( s_pack_internal(soself, args, 0, buf, state) != 0 ) { + PyMutex_Unlock(&soself->mutex); PyBytesWriter_Discard(writer); return NULL; } + PyMutex_Unlock(&soself->mutex); - return PyBytesWriter_FinishWithSize(writer, frozen_size); + return PyBytesWriter_FinishWithSize(writer, soself->s_size); } PyDoc_STRVAR(s_pack_into__doc__, @@ -2378,11 +2375,13 @@ s_pack_into(PyObject *self, PyObject *const *args, Py_ssize_t nargs) } /* Call the guts */ + PyMutex_Lock(&soself->mutex); if (s_pack_internal(soself, args, 2, (char*)buffer.buf + offset, state) != 0) { + PyMutex_Unlock(&soself->mutex); PyBuffer_Release(&buffer); return NULL; } - + PyMutex_Unlock(&soself->mutex); PyBuffer_Release(&buffer); Py_RETURN_NONE; } From ab6216773a1e530ae1a770101519690855b23008 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 10 Jan 2026 05:45:08 +0300 Subject: [PATCH 03/10] + clean the mess --- Modules/_struct.c | 5 ----- 1 file changed, 5 deletions(-) diff --git a/Modules/_struct.c b/Modules/_struct.c index d97f6d4a85835f..eeee5fd68d87a9 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -2266,11 +2266,6 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs) char *buf = PyBytesWriter_GetData(writer); /* Call the guts */ - if (PyMutex_IsLocked(&soself->mutex)) { - PyErr_SetString(PyExc_RuntimeError, "XXX"); - PyBytesWriter_Discard(writer); - return NULL; - } PyMutex_Lock(&soself->mutex); if ( s_pack_internal(soself, args, 0, buf, state) != 0 ) { PyMutex_Unlock(&soself->mutex); From 3b184276507949c047612f33026ebbe5b0cf51d3 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 10 Jan 2026 09:55:52 +0300 Subject: [PATCH 04/10] gh-78724: raise RuntimeError's when calling methods on non-ready Struct()'s Calling the ``Struct.__new__()`` dunder without required argument now is deprecated. Calling the ``Struct.__init__()`` dunder method on initialized object now also is deprecated. --- Lib/test/test_struct.py | 25 +++++++++-- ...6-01-10-10-04-08.gh-issue-78724.xkXfxX.rst | 6 +++ Modules/_struct.c | 44 +++++++++++++++++-- 3 files changed, 67 insertions(+), 8 deletions(-) create mode 100644 Misc/NEWS.d/next/Library/2026-01-10-10-04-08.gh-issue-78724.xkXfxX.rst diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 8811388f4ea143..83a95abb5c0d9b 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -7,6 +7,7 @@ import unittest import struct import sys +import warnings import weakref from test import support @@ -575,7 +576,11 @@ def test_Struct_reinitialization(self): # Struct instance. This test can be used to detect the leak # when running with regrtest -L. s = struct.Struct('i') - s.__init__('ii') + with self.assertWarns(DeprecationWarning): + s.__init__('ii') + with warnings.catch_warnings(): + warnings.simplefilter("error", DeprecationWarning) + self.assertRaises(DeprecationWarning, s.__init__, 'ii') def check_sizeof(self, format_str, number_of_codes): # The size of 'PyStructObject' @@ -783,7 +788,8 @@ class MyStruct(struct.Struct): def __init__(self): super().__init__('>h') - my_struct = MyStruct() + with self.assertWarns(DeprecationWarning): + my_struct = MyStruct() self.assertEqual(my_struct.pack(12345), b'\x30\x39') def test_repr(self): @@ -826,8 +832,19 @@ def __bool__(self): S.__init__('I') return True - self.assertRaises(RuntimeError, S.pack, Evil(), 1) - self.assertRaises(RuntimeError, S.pack_into, buf, 0, Evil(), 1) + with self.assertWarns(DeprecationWarning): + self.assertRaises(RuntimeError, S.pack, Evil(), 1) + self.assertRaises(RuntimeError, S.pack_into, buf, 0, Evil(), 1) + + def test_operations_on_half_initialized_Struct(self): + with self.assertWarns(DeprecationWarning): + S = struct.Struct.__new__(struct.Struct) + + spam = array.array('b', b' ') + for attr in ['iter_unpack', 'pack', 'pack_into', + 'unpack', 'unpack_from']: + meth = getattr(S, attr) + self.assertRaises(RuntimeError, meth, spam) class UnpackIteratorTest(unittest.TestCase): diff --git a/Misc/NEWS.d/next/Library/2026-01-10-10-04-08.gh-issue-78724.xkXfxX.rst b/Misc/NEWS.d/next/Library/2026-01-10-10-04-08.gh-issue-78724.xkXfxX.rst new file mode 100644 index 00000000000000..8665b5b5774905 --- /dev/null +++ b/Misc/NEWS.d/next/Library/2026-01-10-10-04-08.gh-issue-78724.xkXfxX.rst @@ -0,0 +1,6 @@ +Prevent crashes due to using half-initialized :class:`~struct.Struct` +objects. For example, created by ``Struct.__new__(Struct)``. Calling the +``Struct.__new__()`` dunder without required argument now is deprecated. +Calling :meth:`~object.__init__` dunder method of :class:`~struct.Struct` +object on initialized object now also is deprecated. Patch by Sergey B +Kirpichev. diff --git a/Modules/_struct.c b/Modules/_struct.c index eeee5fd68d87a9..d4c9a0ddfb1ae7 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -71,6 +71,7 @@ typedef struct { PyObject *s_format; PyObject *weakreflist; /* List of weak references */ PyMutex mutex; /* to prevent mutation during packing */ + bool ready; } PyStructObject; #define PyStructObject_CAST(op) ((PyStructObject *)(op)) @@ -1763,6 +1764,12 @@ s_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { PyObject *self; + if (PyTuple_GET_SIZE(args) != 1 + && PyErr_WarnEx(PyExc_DeprecationWarning, + "Struct().__new__() has one required argument", 1)) + { + return NULL; + } assert(type != NULL); allocfunc alloc_func = PyType_GetSlot(type, Py_tp_alloc); assert(alloc_func != NULL); @@ -1775,6 +1782,7 @@ s_new(PyTypeObject *type, PyObject *args, PyObject *kwds) s->s_size = -1; s->s_len = -1; s->mutex = (PyMutex){0}; + s->ready = false; } return self; } @@ -1796,8 +1804,13 @@ static int Struct___init___impl(PyStructObject *self, PyObject *format) /*[clinic end generated code: output=b8e80862444e92d0 input=192a4575a3dde802]*/ { - int ret = 0; - + if (self->ready + && PyErr_WarnEx(PyExc_DeprecationWarning, + ("Explicit call of __init__() on " + "initialized Struct()"), 1)) + { + return -1; + } if (PyUnicode_Check(format)) { format = PyUnicode_AsASCIIString(format); if (format == NULL) @@ -1823,8 +1836,11 @@ Struct___init___impl(PyStructObject *self, PyObject *format) "Call Struct.__init__() in struct.pack()"); return -1; } - ret = prepare_s(self); - return ret; + if (prepare_s(self)) { + return -1; + } + self->ready = true; + return 0; } static int @@ -1924,6 +1940,10 @@ Struct_unpack_impl(PyStructObject *self, Py_buffer *buffer) /*[clinic end generated code: output=873a24faf02e848a input=3113f8e7038b2f6c]*/ { _structmodulestate *state = get_struct_state_structinst(self); + if (!self->ready) { + PyErr_SetString(PyExc_RuntimeError, "Call unpack on non-initialized Struct()"); + return NULL; + } assert(self->s_codes != NULL); if (buffer->len != self->s_size) { PyErr_Format(state->StructError, @@ -1956,6 +1976,10 @@ Struct_unpack_from_impl(PyStructObject *self, Py_buffer *buffer, /*[clinic end generated code: output=57fac875e0977316 input=cafd4851d473c894]*/ { _structmodulestate *state = get_struct_state_structinst(self); + if (!self->ready) { + PyErr_SetString(PyExc_RuntimeError, "Call unpack_from on non-initialized Struct()"); + return NULL; + } assert(self->s_codes != NULL); if (offset < 0) { @@ -2108,6 +2132,10 @@ Struct_iter_unpack_impl(PyStructObject *self, PyObject *buffer) { _structmodulestate *state = get_struct_state_structinst(self); unpackiterobject *iter; + if (!self->ready) { + PyErr_SetString(PyExc_RuntimeError, "Call iter_unpack on non-initialized Struct()"); + return NULL; + } assert(self->s_codes != NULL); @@ -2249,6 +2277,10 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs) /* Validate arguments. */ soself = PyStructObject_CAST(self); + if (!soself->ready) { + PyErr_SetString(PyExc_RuntimeError, "Call pack on non-initialized Struct()"); + return NULL; + } assert(PyStruct_Check(self, state)); assert(soself->s_codes != NULL); if (nargs != soself->s_len) @@ -2295,6 +2327,10 @@ s_pack_into(PyObject *self, PyObject *const *args, Py_ssize_t nargs) /* Validate arguments. +1 is for the first arg as buffer. */ soself = PyStructObject_CAST(self); + if (!soself->ready) { + PyErr_SetString(PyExc_RuntimeError, "Call pack_into on non-initialized Struct()"); + return NULL; + } assert(PyStruct_Check(self, state)); assert(soself->s_codes != NULL); if (nargs != (soself->s_len + 2)) From b6c8bed6d3bdc50fd62b0cafce1ae7b7b5ac95a6 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 10 Jan 2026 13:21:33 +0300 Subject: [PATCH 05/10] revert stuff from #143382 --- Lib/test/test_struct.py | 14 -------------- ...026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst | 4 ---- Modules/_struct.c | 16 ++-------------- 3 files changed, 2 insertions(+), 32 deletions(-) delete mode 100644 Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 83a95abb5c0d9b..58f32d63c5161e 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -822,20 +822,6 @@ def test_endian_table_init_subinterpreters(self): results = executor.map(exec, [code] * 5) self.assertListEqual(list(results), [None] * 5) - def test_Struct_object_mutation_via_dunders(self): - S = struct.Struct('?I') - buf = array.array('b', b' '*100) - - class Evil(): - def __bool__(self): - # This rebuilds format codes during S.pack(). - S.__init__('I') - return True - - with self.assertWarns(DeprecationWarning): - self.assertRaises(RuntimeError, S.pack, Evil(), 1) - self.assertRaises(RuntimeError, S.pack_into, buf, 0, Evil(), 1) - def test_operations_on_half_initialized_Struct(self): with self.assertWarns(DeprecationWarning): S = struct.Struct.__new__(struct.Struct) diff --git a/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst b/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst deleted file mode 100644 index c2612ef27a558c..00000000000000 --- a/Misc/NEWS.d/next/Library/2026-01-03-10-15-32.gh-issue-143379.iz-hU7.rst +++ /dev/null @@ -1,4 +0,0 @@ -Fix use-after-free in :meth:`struct.Struct.pack` when the -:class:`struct.Struct` object is mutated by dunder methods (like -:meth:`object.__bool__`) during packing of arguments. Now this trigger -:exc:`RuntimeError`. Patch by Sergey B Kirpichev. diff --git a/Modules/_struct.c b/Modules/_struct.c index d4c9a0ddfb1ae7..9a7660d0efe2c8 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -70,7 +70,6 @@ typedef struct { formatcode *s_codes; PyObject *s_format; PyObject *weakreflist; /* List of weak references */ - PyMutex mutex; /* to prevent mutation during packing */ bool ready; } PyStructObject; @@ -1781,7 +1780,6 @@ s_new(PyTypeObject *type, PyObject *args, PyObject *kwds) s->s_codes = NULL; s->s_size = -1; s->s_len = -1; - s->mutex = (PyMutex){0}; s->ready = false; } return self; @@ -1831,11 +1829,6 @@ Struct___init___impl(PyStructObject *self, PyObject *format) Py_SETREF(self->s_format, format); - if (PyMutex_IsLocked(&self->mutex)) { - PyErr_SetString(PyExc_RuntimeError, - "Call Struct.__init__() in struct.pack()"); - return -1; - } if (prepare_s(self)) { return -1; } @@ -2174,7 +2167,7 @@ Struct_iter_unpack_impl(PyStructObject *self, PyObject *buffer) * argument for where to start processing the arguments for packing, and a * character buffer for writing the packed string. The caller must insure * that the buffer may contain the required length for packing the arguments. - * 0 is returned on success, -1 is returned if there is an error. + * 0 is returned on success, 1 is returned if there is an error. * */ static int @@ -2298,13 +2291,10 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs) char *buf = PyBytesWriter_GetData(writer); /* Call the guts */ - PyMutex_Lock(&soself->mutex); if ( s_pack_internal(soself, args, 0, buf, state) != 0 ) { - PyMutex_Unlock(&soself->mutex); PyBytesWriter_Discard(writer); return NULL; } - PyMutex_Unlock(&soself->mutex); return PyBytesWriter_FinishWithSize(writer, soself->s_size); } @@ -2406,13 +2396,11 @@ s_pack_into(PyObject *self, PyObject *const *args, Py_ssize_t nargs) } /* Call the guts */ - PyMutex_Lock(&soself->mutex); if (s_pack_internal(soself, args, 2, (char*)buffer.buf + offset, state) != 0) { - PyMutex_Unlock(&soself->mutex); PyBuffer_Release(&buffer); return NULL; } - PyMutex_Unlock(&soself->mutex); + PyBuffer_Release(&buffer); Py_RETURN_NONE; } From f542173235a32e8d9dadc84b7ca7bc2af2f9a585 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 10 Jan 2026 13:26:29 +0300 Subject: [PATCH 06/10] revert deprecations (TBD: a separate pr) --- Lib/test/test_struct.py | 13 +++---------- Modules/_struct.c | 13 ------------- 2 files changed, 3 insertions(+), 23 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index 58f32d63c5161e..defef9f88bfab2 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -7,7 +7,6 @@ import unittest import struct import sys -import warnings import weakref from test import support @@ -576,11 +575,7 @@ def test_Struct_reinitialization(self): # Struct instance. This test can be used to detect the leak # when running with regrtest -L. s = struct.Struct('i') - with self.assertWarns(DeprecationWarning): - s.__init__('ii') - with warnings.catch_warnings(): - warnings.simplefilter("error", DeprecationWarning) - self.assertRaises(DeprecationWarning, s.__init__, 'ii') + s.__init__('ii') def check_sizeof(self, format_str, number_of_codes): # The size of 'PyStructObject' @@ -788,8 +783,7 @@ class MyStruct(struct.Struct): def __init__(self): super().__init__('>h') - with self.assertWarns(DeprecationWarning): - my_struct = MyStruct() + my_struct = MyStruct() self.assertEqual(my_struct.pack(12345), b'\x30\x39') def test_repr(self): @@ -823,8 +817,7 @@ def test_endian_table_init_subinterpreters(self): self.assertListEqual(list(results), [None] * 5) def test_operations_on_half_initialized_Struct(self): - with self.assertWarns(DeprecationWarning): - S = struct.Struct.__new__(struct.Struct) + S = struct.Struct.__new__(struct.Struct) spam = array.array('b', b' ') for attr in ['iter_unpack', 'pack', 'pack_into', diff --git a/Modules/_struct.c b/Modules/_struct.c index 9a7660d0efe2c8..87db03469e1723 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -1763,12 +1763,6 @@ s_new(PyTypeObject *type, PyObject *args, PyObject *kwds) { PyObject *self; - if (PyTuple_GET_SIZE(args) != 1 - && PyErr_WarnEx(PyExc_DeprecationWarning, - "Struct().__new__() has one required argument", 1)) - { - return NULL; - } assert(type != NULL); allocfunc alloc_func = PyType_GetSlot(type, Py_tp_alloc); assert(alloc_func != NULL); @@ -1802,13 +1796,6 @@ static int Struct___init___impl(PyStructObject *self, PyObject *format) /*[clinic end generated code: output=b8e80862444e92d0 input=192a4575a3dde802]*/ { - if (self->ready - && PyErr_WarnEx(PyExc_DeprecationWarning, - ("Explicit call of __init__() on " - "initialized Struct()"), 1)) - { - return -1; - } if (PyUnicode_Check(format)) { format = PyUnicode_AsASCIIString(format); if (format == NULL) From 69a5eecfe47a04b964265a8c2abe5bd5d57f2bc8 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 10 Jan 2026 14:39:45 +0300 Subject: [PATCH 07/10] address review: redo patch to use s_codes --- Lib/test/test_struct.py | 2 +- ...6-01-10-10-04-08.gh-issue-78724.xkXfxX.rst | 9 ++--- Modules/_struct.c | 40 +++++++------------ 3 files changed, 19 insertions(+), 32 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index defef9f88bfab2..ec4881aa3df19a 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -579,7 +579,7 @@ def test_Struct_reinitialization(self): def check_sizeof(self, format_str, number_of_codes): # The size of 'PyStructObject' - totalsize = support.calcobjsize('2n3PB') + totalsize = support.calcobjsize('2n3P') # The size taken up by the 'formatcode' dynamic array totalsize += struct.calcsize('P3n0P') * (number_of_codes + 1) support.check_sizeof(self, struct.Struct(format_str), totalsize) diff --git a/Misc/NEWS.d/next/Library/2026-01-10-10-04-08.gh-issue-78724.xkXfxX.rst b/Misc/NEWS.d/next/Library/2026-01-10-10-04-08.gh-issue-78724.xkXfxX.rst index 8665b5b5774905..8a4bec4e1653d1 100644 --- a/Misc/NEWS.d/next/Library/2026-01-10-10-04-08.gh-issue-78724.xkXfxX.rst +++ b/Misc/NEWS.d/next/Library/2026-01-10-10-04-08.gh-issue-78724.xkXfxX.rst @@ -1,6 +1,3 @@ -Prevent crashes due to using half-initialized :class:`~struct.Struct` -objects. For example, created by ``Struct.__new__(Struct)``. Calling the -``Struct.__new__()`` dunder without required argument now is deprecated. -Calling :meth:`~object.__init__` dunder method of :class:`~struct.Struct` -object on initialized object now also is deprecated. Patch by Sergey B -Kirpichev. +Raise :exc:`RuntimeError`'s when user attempts to call methods on +half-initialized :class:`~struct.Struct` objects, For example, created by +``Struct.__new__(Struct)``. Patch by Sergey B Kirpichev. diff --git a/Modules/_struct.c b/Modules/_struct.c index 87db03469e1723..897f6df7015e14 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -70,7 +70,6 @@ typedef struct { formatcode *s_codes; PyObject *s_format; PyObject *weakreflist; /* List of weak references */ - bool ready; } PyStructObject; #define PyStructObject_CAST(op) ((PyStructObject *)(op)) @@ -1699,8 +1698,6 @@ prepare_s(PyStructObject *self) return -1; } - self->s_size = size; - self->s_len = len; codes = PyMem_Malloc((ncodes + 1) * sizeof(formatcode)); if (codes == NULL) { PyErr_NoMemory(); @@ -1710,6 +1707,8 @@ prepare_s(PyStructObject *self) if (self->s_codes != NULL) PyMem_Free(self->s_codes); self->s_codes = codes; + self->s_size = size; + self->s_len = len; s = fmt; size = 0; @@ -1774,7 +1773,6 @@ s_new(PyTypeObject *type, PyObject *args, PyObject *kwds) s->s_codes = NULL; s->s_size = -1; s->s_len = -1; - s->ready = false; } return self; } @@ -1819,7 +1817,6 @@ Struct___init___impl(PyStructObject *self, PyObject *format) if (prepare_s(self)) { return -1; } - self->ready = true; return 0; } @@ -1900,6 +1897,14 @@ s_unpack_internal(PyStructObject *soself, const char *startfrom, return NULL; } +#define ENSURE_STRUCT_IS_READY(self) \ + do { \ + if (!(self)->s_codes) { \ + PyErr_SetString(PyExc_RuntimeError, \ + "Struct object is not initialized"); \ + return NULL; \ + } \ + } while (0); /*[clinic input] Struct.unpack @@ -1920,10 +1925,7 @@ Struct_unpack_impl(PyStructObject *self, Py_buffer *buffer) /*[clinic end generated code: output=873a24faf02e848a input=3113f8e7038b2f6c]*/ { _structmodulestate *state = get_struct_state_structinst(self); - if (!self->ready) { - PyErr_SetString(PyExc_RuntimeError, "Call unpack on non-initialized Struct()"); - return NULL; - } + ENSURE_STRUCT_IS_READY(self); assert(self->s_codes != NULL); if (buffer->len != self->s_size) { PyErr_Format(state->StructError, @@ -1956,10 +1958,7 @@ Struct_unpack_from_impl(PyStructObject *self, Py_buffer *buffer, /*[clinic end generated code: output=57fac875e0977316 input=cafd4851d473c894]*/ { _structmodulestate *state = get_struct_state_structinst(self); - if (!self->ready) { - PyErr_SetString(PyExc_RuntimeError, "Call unpack_from on non-initialized Struct()"); - return NULL; - } + ENSURE_STRUCT_IS_READY(self); assert(self->s_codes != NULL); if (offset < 0) { @@ -2112,10 +2111,7 @@ Struct_iter_unpack_impl(PyStructObject *self, PyObject *buffer) { _structmodulestate *state = get_struct_state_structinst(self); unpackiterobject *iter; - if (!self->ready) { - PyErr_SetString(PyExc_RuntimeError, "Call iter_unpack on non-initialized Struct()"); - return NULL; - } + ENSURE_STRUCT_IS_READY(self); assert(self->s_codes != NULL); @@ -2257,10 +2253,7 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs) /* Validate arguments. */ soself = PyStructObject_CAST(self); - if (!soself->ready) { - PyErr_SetString(PyExc_RuntimeError, "Call pack on non-initialized Struct()"); - return NULL; - } + ENSURE_STRUCT_IS_READY(soself); assert(PyStruct_Check(self, state)); assert(soself->s_codes != NULL); if (nargs != soself->s_len) @@ -2304,10 +2297,7 @@ s_pack_into(PyObject *self, PyObject *const *args, Py_ssize_t nargs) /* Validate arguments. +1 is for the first arg as buffer. */ soself = PyStructObject_CAST(self); - if (!soself->ready) { - PyErr_SetString(PyExc_RuntimeError, "Call pack_into on non-initialized Struct()"); - return NULL; - } + ENSURE_STRUCT_IS_READY(soself); assert(PyStruct_Check(self, state)); assert(soself->s_codes != NULL); if (nargs != (soself->s_len + 2)) From 8ba6b3347ae5b5bb8255470a32ee0f89cf0d9529 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sat, 10 Jan 2026 17:03:09 +0300 Subject: [PATCH 08/10] address review: -asserts + redo test --- Lib/test/test_struct.py | 11 +++++++---- Modules/_struct.c | 7 +------ 2 files changed, 8 insertions(+), 10 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index ec4881aa3df19a..be6326bbdfc21d 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -820,10 +820,13 @@ def test_operations_on_half_initialized_Struct(self): S = struct.Struct.__new__(struct.Struct) spam = array.array('b', b' ') - for attr in ['iter_unpack', 'pack', 'pack_into', - 'unpack', 'unpack_from']: - meth = getattr(S, attr) - self.assertRaises(RuntimeError, meth, spam) + self.assertRaises(RuntimeError, S.iter_unpack, 1) + self.assertRaises(RuntimeError, S.pack, 1) + self.assertRaises(RuntimeError, S.pack_into, 1) + self.assertRaises(RuntimeError, S.unpack, spam) + self.assertRaises(RuntimeError, S.unpack_from, spam) + self.assertRaises(RuntimeError, getattr, S, 'format') + self.assertEqual(S.size, -1) class UnpackIteratorTest(unittest.TestCase): diff --git a/Modules/_struct.c b/Modules/_struct.c index 897f6df7015e14..aa28f8003d43ff 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -1926,7 +1926,6 @@ Struct_unpack_impl(PyStructObject *self, Py_buffer *buffer) { _structmodulestate *state = get_struct_state_structinst(self); ENSURE_STRUCT_IS_READY(self); - assert(self->s_codes != NULL); if (buffer->len != self->s_size) { PyErr_Format(state->StructError, "unpack requires a buffer of %zd bytes", @@ -1959,7 +1958,6 @@ Struct_unpack_from_impl(PyStructObject *self, Py_buffer *buffer, { _structmodulestate *state = get_struct_state_structinst(self); ENSURE_STRUCT_IS_READY(self); - assert(self->s_codes != NULL); if (offset < 0) { if (offset + self->s_size > 0) { @@ -2113,8 +2111,6 @@ Struct_iter_unpack_impl(PyStructObject *self, PyObject *buffer) unpackiterobject *iter; ENSURE_STRUCT_IS_READY(self); - assert(self->s_codes != NULL); - if (self->s_size == 0) { PyErr_Format(state->StructError, "cannot iteratively unpack with a struct of length 0"); @@ -2255,7 +2251,6 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs) soself = PyStructObject_CAST(self); ENSURE_STRUCT_IS_READY(soself); assert(PyStruct_Check(self, state)); - assert(soself->s_codes != NULL); if (nargs != soself->s_len) { PyErr_Format(state->StructError, @@ -2299,7 +2294,6 @@ s_pack_into(PyObject *self, PyObject *const *args, Py_ssize_t nargs) soself = PyStructObject_CAST(self); ENSURE_STRUCT_IS_READY(soself); assert(PyStruct_Check(self, state)); - assert(soself->s_codes != NULL); if (nargs != (soself->s_len + 2)) { if (nargs == 0) { @@ -2386,6 +2380,7 @@ static PyObject * s_get_format(PyObject *op, void *Py_UNUSED(closure)) { PyStructObject *self = PyStructObject_CAST(op); + ENSURE_STRUCT_IS_READY(self); return PyUnicode_FromStringAndSize(PyBytes_AS_STRING(self->s_format), PyBytes_GET_SIZE(self->s_format)); } From ea0f8e202a974a5717b9f06c5be130553a859f3e Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sun, 11 Jan 2026 01:48:16 +0300 Subject: [PATCH 09/10] address review: adjust tests --- Lib/test/test_struct.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Lib/test/test_struct.py b/Lib/test/test_struct.py index be6326bbdfc21d..88662fec60fe4a 100644 --- a/Lib/test/test_struct.py +++ b/Lib/test/test_struct.py @@ -820,9 +820,9 @@ def test_operations_on_half_initialized_Struct(self): S = struct.Struct.__new__(struct.Struct) spam = array.array('b', b' ') - self.assertRaises(RuntimeError, S.iter_unpack, 1) + self.assertRaises(RuntimeError, S.iter_unpack, spam) self.assertRaises(RuntimeError, S.pack, 1) - self.assertRaises(RuntimeError, S.pack_into, 1) + self.assertRaises(RuntimeError, S.pack_into, spam, 1) self.assertRaises(RuntimeError, S.unpack, spam) self.assertRaises(RuntimeError, S.unpack_from, spam) self.assertRaises(RuntimeError, getattr, S, 'format') From b61bddb41b82322ed3de8b4636c41312c5bbb8b5 Mon Sep 17 00:00:00 2001 From: Sergey B Kirpichev Date: Sun, 11 Jan 2026 18:23:17 +0300 Subject: [PATCH 10/10] address review: revert unrelated change --- Modules/_struct.c | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/Modules/_struct.c b/Modules/_struct.c index aa28f8003d43ff..a8e9021f0a303a 100644 --- a/Modules/_struct.c +++ b/Modules/_struct.c @@ -1794,6 +1794,8 @@ static int Struct___init___impl(PyStructObject *self, PyObject *format) /*[clinic end generated code: output=b8e80862444e92d0 input=192a4575a3dde802]*/ { + int ret = 0; + if (PyUnicode_Check(format)) { format = PyUnicode_AsASCIIString(format); if (format == NULL) @@ -1814,10 +1816,8 @@ Struct___init___impl(PyStructObject *self, PyObject *format) Py_SETREF(self->s_format, format); - if (prepare_s(self)) { - return -1; - } - return 0; + ret = prepare_s(self); + return ret; } static int