Skip to content
3 changes: 3 additions & 0 deletions Include/internal/pycore_pyatomic_ft_wrappers.h
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,8 @@ extern "C" {
_Py_atomic_store_ulong_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) \
_Py_atomic_store_ssize_relaxed(&value, new_value)
#define FT_ATOMIC_STORE_SSIZE(value, new_value) \
_Py_atomic_store_ssize(&value, new_value)
#define FT_ATOMIC_STORE_FLOAT_RELAXED(value, new_value) \
_Py_atomic_store_float_relaxed(&value, new_value)
#define FT_ATOMIC_LOAD_FLOAT_RELAXED(value) \
Expand Down Expand Up @@ -151,6 +153,7 @@ extern "C" {
#define FT_ATOMIC_STORE_INT8_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_INT8_RELEASE(value, new_value) value = new_value
#define FT_ATOMIC_STORE_SSIZE_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_SSIZE(value, new_value) value = new_value
#define FT_ATOMIC_STORE_SSIZE_RELEASE(value, new_value) value = new_value
#define FT_ATOMIC_STORE_UINT8_RELAXED(value, new_value) value = new_value
#define FT_ATOMIC_STORE_UINT16_RELAXED(value, new_value) value = new_value
Expand Down
15 changes: 14 additions & 1 deletion Lib/test/test_struct.py
Original file line number Diff line number Diff line change
Expand Up @@ -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('2n3P1n')
# 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)
Expand Down Expand Up @@ -816,6 +816,19 @@ 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

self.assertRaises(RuntimeError, S.pack, Evil(), 1)
self.assertRaises(RuntimeError, S.pack_into, buf, 0, Evil(), 1)


class UnpackIteratorTest(unittest.TestCase):
"""
Expand Down
Original file line number Diff line number Diff line change
@@ -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. Now this trigger
:exc:`RuntimeError`. Patch by Sergey B Kirpichev.
19 changes: 18 additions & 1 deletion Modules/_struct.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ typedef struct {
formatcode *s_codes;
PyObject *s_format;
PyObject *weakreflist; /* List of weak references */
Py_ssize_t mutex_cnt; /* to prevent mutation during packing */
} PyStructObject;

#define PyStructObject_CAST(op) ((PyStructObject *)(op))
Expand Down Expand Up @@ -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_cnt = 0;
}
return self;
}
Expand Down Expand Up @@ -1816,6 +1818,11 @@ Struct___init___impl(PyStructObject *self, PyObject *format)

Py_SETREF(self->s_format, format);

if (FT_ATOMIC_LOAD_SSIZE(self->mutex_cnt)) {
PyErr_SetString(PyExc_RuntimeError,
"Call Struct.__init__() in struct.pack()");
return -1;
}
ret = prepare_s(self);
return ret;
}
Expand Down Expand Up @@ -2139,7 +2146,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
Expand Down Expand Up @@ -2259,10 +2266,15 @@ s_pack(PyObject *self, PyObject *const *args, Py_ssize_t nargs)
char *buf = PyBytesWriter_GetData(writer);

/* Call the guts */
Py_ssize_t prev_cnt = FT_ATOMIC_LOAD_SSIZE(soself->mutex_cnt);

FT_ATOMIC_ADD_SSIZE(soself->mutex_cnt, 1);
if ( s_pack_internal(soself, args, 0, buf, state) != 0 ) {
FT_ATOMIC_STORE_SSIZE(soself->mutex_cnt, prev_cnt);
PyBytesWriter_Discard(writer);
return NULL;
}
FT_ATOMIC_STORE_SSIZE(soself->mutex_cnt, prev_cnt);

return PyBytesWriter_FinishWithSize(writer, soself->s_size);
}
Expand Down Expand Up @@ -2360,10 +2372,15 @@ s_pack_into(PyObject *self, PyObject *const *args, Py_ssize_t nargs)
}

/* Call the guts */
Py_ssize_t prev_cnt = FT_ATOMIC_LOAD_SSIZE(soself->mutex_cnt);

FT_ATOMIC_ADD_SSIZE(soself->mutex_cnt, 1);
if (s_pack_internal(soself, args, 2, (char*)buffer.buf + offset, state) != 0) {
FT_ATOMIC_STORE_SSIZE(soself->mutex_cnt, prev_cnt);
PyBuffer_Release(&buffer);
return NULL;
}
FT_ATOMIC_STORE_SSIZE(soself->mutex_cnt, prev_cnt);

PyBuffer_Release(&buffer);
Py_RETURN_NONE;
Expand Down
Loading