From 664ddc99ce16428392cd46f2ce3b44bdace36b67 Mon Sep 17 00:00:00 2001 From: abhinavmir Date: Sun, 30 Aug 2026 00:42:04 -0700 Subject: [PATCH] Reject a mode change while an object is incomplete Unpacker.skip() does not build the objects on the parser stack. So stack[].obj stays uninitialized after an incomplete skip(). A later Unpacker.unpack() writes an array item through that pointer and the process crashes. The reverse order drops the reference that the construct pass took. Record the mode that started the object in unpack_context. unpack_construct() and unpack_skip() now raise ValueError on a change. The parser state stays intact, so the original mode still finishes the object. --- msgpack/unpack_template.h | 26 ++++++++++++++++++++++++++ test/test_sequnpack.py | 29 ++++++++++++++++++++++++++++- 2 files changed, 54 insertions(+), 1 deletion(-) diff --git a/msgpack/unpack_template.h b/msgpack/unpack_template.h index 797a2f91..cb4daa6a 100644 --- a/msgpack/unpack_template.h +++ b/msgpack/unpack_template.h @@ -35,6 +35,8 @@ struct unpack_context { unsigned int cs; unsigned int trail; unsigned int top; + /* mode that started the object on the stack. See unpack_check_mode(). */ + bool construct; unpack_stack stack[MSGPACK_EMBED_STACK_SIZE]; }; @@ -44,6 +46,7 @@ static inline void unpack_init(unpack_context* ctx) ctx->cs = CS_HEADER; ctx->trail = 0; ctx->top = 0; + ctx->construct = true; ctx->stack[0].obj = NULL; } @@ -386,7 +389,27 @@ static inline int unpack_execute(bool construct, unpack_context* ctx, const char #undef again_fixed_trail_if_zero #undef start_container +/* + * skip mode does not build the objects on the stack, so stack[].obj stays + * uninitialized. A later construct pass writes an item through that pointer. + * The reverse order drops the reference that the construct pass took. + * Reject the mode change while an object is still open. + */ +static inline int unpack_check_mode(unpack_context *ctx, bool construct) +{ + if (ctx->top != 0 && ctx->construct != construct) { + PyErr_SetString(PyExc_ValueError, + "cannot switch between unpack and skip while an object is incomplete"); + return -1; + } + ctx->construct = construct; + return 0; +} + static int unpack_construct(unpack_context *ctx, const char *data, Py_ssize_t len, Py_ssize_t *off) { + if (unpack_check_mode(ctx, true) < 0) { + return -1; + } int ret = unpack_execute(1, ctx, data, len, off); if (ret == -1) { unpack_clear(ctx); @@ -394,6 +417,9 @@ static int unpack_construct(unpack_context *ctx, const char *data, Py_ssize_t le return ret; } static int unpack_skip(unpack_context *ctx, const char *data, Py_ssize_t len, Py_ssize_t *off) { + if (unpack_check_mode(ctx, false) < 0) { + return -1; + } int ret = unpack_execute(0, ctx, data, len, off); if (ret == -1) { unpack_clear(ctx); diff --git a/test/test_sequnpack.py b/test/test_sequnpack.py index 0f895d7d..d9d56a83 100644 --- a/test/test_sequnpack.py +++ b/test/test_sequnpack.py @@ -1,7 +1,7 @@ #!/usr/bin/env python import io -from pytest import raises +from pytest import mark, raises from msgpack import BufferFull, Unpacker, pack, packb from msgpack.exceptions import OutOfData @@ -146,3 +146,30 @@ def test_unpack_tell(): m2 = next(unpacker) assert m == m2 assert o == unpacker.tell() + + +@mark.skipif( + Unpacker.__module__ == "msgpack.fallback", + reason="only the C extension keeps parser state between calls", +) +def test_mode_switch_while_incomplete(): + # skip() does not build the objects that unpack() needs, so the parser + # state of one mode is not valid for the other. The unpacker must reject + # the change instead of a crash. The original mode still finishes. + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.skip() + unpacker.feed(b"\x00") + with raises(ValueError): + unpacker.unpack() + assert unpacker.skip() is None + + unpacker = Unpacker() + unpacker.feed(b"\x91") + with raises(OutOfData): + unpacker.unpack() + unpacker.feed(b"\x00") + with raises(ValueError): + unpacker.skip() + assert unpacker.unpack() == [0]