From b08617d6a59c6c2ce18a97bfd54c551a9db53794 Mon Sep 17 00:00:00 2001 From: CAOShurong <170531907+CAOShurong@users.noreply.github.com> Date: Thu, 5 Nov 2026 21:14:19 +0800 Subject: [PATCH] Refuse to export the Packer buffer during pack The C Packer accepted getbuffer() from a default callback, from dict.items(), and from pack_map_pairs() while it was still writing. Growing the buffer then freed the exported memory. Those methods now reject the export until they return. pytest -v test: 181 passed, 1 skipped. MSGPACK_PUREPYTHON=1 pytest -v test: 178 passed, 4 skipped. The pure Python buffer is not freed when it grows. --- msgpack/_packer.pyx | 80 +++++++++++++++++++++++++++++---------------- test/test_buffer.py | 61 ++++++++++++++++++++++++++++++++++ 2 files changed, 112 insertions(+), 29 deletions(-) diff --git a/msgpack/_packer.pyx b/msgpack/_packer.pyx index c44c6366..07481f42 100644 --- a/msgpack/_packer.pyx +++ b/msgpack/_packer.pyx @@ -106,6 +106,7 @@ cdef class Packer: cdef object _berrors cdef const char *unicode_errors cdef size_t exports # number of exported buffers + cdef int _packing # public pack methods currently running cdef bint strict_types cdef bint use_float cdef bint autoreset @@ -118,6 +119,7 @@ cdef class Packer: self.pk.buf_size = buf_size self.pk.length = 0 self.exports = 0 + self._packing = 0 def __dealloc__(self): PyMem_Free(self.pk.buf) @@ -273,29 +275,40 @@ cdef class Packer: def pack(self, object obj): cdef int ret self._check_exports() + # User code runs inside _pack (default, items(), iteration) and can + # export this buffer. Hold the flag until every write from this call + # has finished, including a nested pack. + self._packing += 1 try: - ret = self._pack(obj, DEFAULT_RECURSE_LIMIT) - except: - self.pk.length = 0 - raise - if ret: # should not happen. - raise RuntimeError("internal error") - if self.autoreset: - buf = PyBytes_FromStringAndSize(self.pk.buf, self.pk.length) - self.pk.length = 0 - return buf + try: + ret = self._pack(obj, DEFAULT_RECURSE_LIMIT) + except: + self.pk.length = 0 + raise + if ret: # should not happen. + raise RuntimeError("internal error") + if self.autoreset: + buf = PyBytes_FromStringAndSize(self.pk.buf, self.pk.length) + self.pk.length = 0 + return buf + finally: + self._packing -= 1 @cython.critical_section def pack_ext_type(self, typecode, data): self._check_exports() - if len(data) > ITEM_LIMIT: - raise ValueError("ext data too large") - msgpack_pack_ext(&self.pk, typecode, len(data)) - msgpack_pack_raw_body(&self.pk, data, len(data)) - if self.autoreset: - buf = PyBytes_FromStringAndSize(self.pk.buf, self.pk.length) - self.pk.length = 0 - return buf + self._packing += 1 + try: + if len(data) > ITEM_LIMIT: + raise ValueError("ext data too large") + msgpack_pack_ext(&self.pk, typecode, len(data)) + msgpack_pack_raw_body(&self.pk, data, len(data)) + if self.autoreset: + buf = PyBytes_FromStringAndSize(self.pk.buf, self.pk.length) + self.pk.length = 0 + return buf + finally: + self._packing -= 1 @cython.critical_section def pack_array_header(self, long long size): @@ -332,17 +345,21 @@ cdef class Packer: (`len(pairs)` and `for k, v in pairs:` should be supported.) """ self._check_exports() - size = len(pairs) - if size > ITEM_LIMIT: - raise ValueError("map too large") - msgpack_pack_map(&self.pk, size) - for k, v in pairs: - self._pack(k) - self._pack(v) - if self.autoreset: - buf = PyBytes_FromStringAndSize(self.pk.buf, self.pk.length) - self.pk.length = 0 - return buf + self._packing += 1 + try: + size = len(pairs) + if size > ITEM_LIMIT: + raise ValueError("map too large") + msgpack_pack_map(&self.pk, size) + for k, v in pairs: + self._pack(k) + self._pack(v) + if self.autoreset: + buf = PyBytes_FromStringAndSize(self.pk.buf, self.pk.length) + self.pk.length = 0 + return buf + finally: + self._packing -= 1 @cython.critical_section def reset(self): @@ -362,11 +379,16 @@ cdef class Packer: """Return memoryview of internal buffer. Note: Packer now supports buffer protocol. You can use memoryview(packer). + Raises BufferError while a pack method is still running. """ return memoryview(self) @cython.critical_section def __getbuffer__(self, Py_buffer *buffer, int flags): + if self._packing: + raise BufferError( + "Packer cannot export its buffer while packing is in progress" + ) PyBuffer_FillInfo(buffer, self, self.pk.buf, self.pk.length, 1, flags) self.exports += 1 diff --git a/test/test_buffer.py b/test/test_buffer.py index ca097222..e5ff46ab 100644 --- a/test/test_buffer.py +++ b/test/test_buffer.py @@ -47,3 +47,64 @@ def test_packer_getbuffer(): buffer.release() packer.pack(42) assert bytes(packer) == b"\x92*\xa5hello*" + + +def _c_extension(): + return Packer.__module__ == "msgpack._cmsgpack" + + +def test_getbuffer_during_default_is_rejected(): + # The default callback runs in the middle of pack(). Exporting the + # internal buffer there lets a later realloc free the view. + if not _c_extension(): + return + + class Unsupported: + pass + + def default(obj): + packer.getbuffer() + return b"ok" + + packer = Packer(default=default, autoreset=False) + with raises(BufferError): + packer.pack(Unsupported()) + packer.pack(b"ok") + assert bytes(packer.getbuffer()) == b"\xc4\x02ok" + + +def test_getbuffer_during_dict_items_is_rejected(): + if not _c_extension(): + return + + packer = Packer(autoreset=False) + + class EvilDict(dict): + def items(self): + packer.getbuffer() + return [(b"k", b"v")] + + with raises(BufferError): + packer.pack(EvilDict({b"x": 1})) + assert bytes(packer.getbuffer()) == b"" + + +def test_getbuffer_during_map_pairs_is_rejected(): + if not _c_extension(): + return + + packer = Packer(autoreset=False) + + class EvilPairs: + def __len__(self): + return 1 + + def __iter__(self): + packer.getbuffer() + yield (b"k", b"v") + + with raises(BufferError): + packer.pack_map_pairs(EvilPairs()) + packer.reset() + packer.pack(b"ok") + assert bytes(packer.bytes()) == b"\xc4\x02ok"