Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Lib/test/test_struct.py
Original file line number Diff line number Diff line change
Expand Up @@ -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):
"""
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. Patch by Sergey B
Kirpichev.
34 changes: 26 additions & 8 deletions Modules/_struct.c
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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);
Expand All @@ -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);
Expand All @@ -2215,15 +2225,20 @@ 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;
}
}

/* Success */
PyMem_Free(frozen_codes);
return 0;
err:
/* Failure */
PyMem_Free(frozen_codes);
return -1;
}


Expand Down Expand Up @@ -2257,14 +2272,17 @@ 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 ) {
PyBytesWriter_Discard(writer);
return NULL;
}

return PyBytesWriter_FinishWithSize(writer, soself->s_size);
return PyBytesWriter_FinishWithSize(writer, frozen_size);
}

PyDoc_STRVAR(s_pack_into__doc__,
Expand Down
Loading