Skip to content
Open
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
12 changes: 6 additions & 6 deletions msgpack/_packer.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ cdef extern from "pack.h":
size_t length
size_t buf_size
bint use_bin_type
size_t exports

int msgpack_pack_nil(msgpack_packer* pk) except -1
int msgpack_pack_true(msgpack_packer* pk) except -1
Expand Down Expand Up @@ -105,7 +106,6 @@ cdef class Packer:
cdef object _default
cdef object _berrors
cdef const char *unicode_errors
cdef size_t exports # number of exported buffers
cdef bint strict_types
cdef bint use_float
cdef bint autoreset
Expand All @@ -117,15 +117,15 @@ cdef class Packer:
raise MemoryError("Unable to allocate internal buffer.")
self.pk.buf_size = buf_size
self.pk.length = 0
self.exports = 0
self.pk.exports = 0

def __dealloc__(self):
PyMem_Free(self.pk.buf)
self.pk.buf = NULL
assert self.exports == 0
assert self.pk.exports == 0

cdef _check_exports(self):
if self.exports > 0:
if self.pk.exports > 0:
raise BufferError("Existing exports of data: Packer cannot be changed")

@cython.critical_section
Expand Down Expand Up @@ -364,8 +364,8 @@ cdef class Packer:
@cython.critical_section
def __getbuffer__(self, Py_buffer *buffer, int flags):
PyBuffer_FillInfo(buffer, self, self.pk.buf, self.pk.length, 1, flags)
self.exports += 1
self.pk.exports += 1

@cython.critical_section
def __releasebuffer__(self, Py_buffer *buffer):
self.exports -= 1
self.pk.exports -= 1
11 changes: 11 additions & 0 deletions msgpack/pack.h
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@ typedef struct msgpack_packer {
size_t length;
size_t buf_size;
bool use_bin_type;
size_t exports;
} msgpack_packer;

typedef struct Packer Packer;
Expand All @@ -43,6 +44,16 @@ static inline int msgpack_pack_write(msgpack_packer* pk, const char *data, size_
size_t len = pk->length;

if (len + l > bs) {
if (pk->exports > 0) {
/* A `default` callback (or anything else running mid-pack) holds a
* live memoryview onto this buffer via getbuffer(). Growing the
* buffer here would realloc it out from under that memoryview,
* since PyMem_Realloc is free to move the allocation, leaving the
* export pointing at freed memory. */
PyErr_SetString(PyExc_BufferError,
"Existing exports of data: cannot resize packer's internal buffer");
return -1;
}
bs = (len + l) * 2;
buf = (char*)PyMem_Realloc(buf, bs);
if (!buf) {
Expand Down
28 changes: 27 additions & 1 deletion test/test_pack.py
Original file line number Diff line number Diff line change
Expand Up @@ -203,8 +203,34 @@ def test_get_buffer():

@pytest.mark.skipif(
Packer.__module__ == "msgpack.fallback",
reason="buf_size only allocates in the C extension",
reason="the fallback packer's getbuffer() returns a BytesIO view, not a view onto a reallocatable C buffer",
)
def test_pack_growth_rejected_while_buffer_exported():
# A default() callback that calls getbuffer() mid-pack holds a live
# memoryview onto the packer's internal buffer. If the packer then needs
# to grow that buffer to fit more data, realloc() is free to move the
# allocation, leaving the export pointing at freed memory (a
# heap-use-after-free once anything reads through it).
exported = []

class Unsupported:
pass

def default(obj):
exported.append(packer.getbuffer())
# Comfortably bigger than buf_size, so packing this forces a realloc.
return b"x" * 4096

packer = Packer(default=default, autoreset=False, buf_size=64)

with pytest.raises(BufferError):
packer.pack(Unsupported())

# The export is still alive and still backed by real memory; releasing it
# shouldn't touch anything that was already freed.
del exported[:]


def test_buf_size_is_converted_once():
# Asking twice let the allocation and the recorded capacity disagree,
# so the packer overflowed a buffer smaller than the size it recorded.
Expand Down
Loading