fix: avoid memory leak when decoding invalid nested arrays by KowalskiThomas · Pull Request #671 · msgpack/msgpack-python

I obviously have less knowledge about this than you so let me know if I'm wrong, but my understanding is that unpack_init sets top to 0 but still pushes something into stack[0]:

As a result, if we looped from [0; top) and top == 0 then we wouldn't free stack[0].obj as far as I can tell.

Regarding map_key at stack[0]: I think we may need to free it depending on the case. If top is 0 then we never need to free it; if top >= 1 then we would need to check for CT_MAP_KEY like we do in the loop.

So bottomline: we can probably iterate from 0 to ctx->top > 0 ? ctx->top : 1 to capture both cases. On top of that, we would need to check whether ctx->top == 0 before the CT_MAP_KEY check (since as far as I can tell, ctx->stack[0].ct would be uninitialised so if we're unlucky, we could accidentally call Py_CLEAR(ctx->stack[0].map_key) which would not be safe.

But if we're adding all that logic, what may actually be simpler is a mix of all the things:

static inline void unpack_clear(unpack_context *ctx)
{
    unsigned int i;
    // The loop captures the case where we did push at least one thing to the stack
    for (i = 0; i < ctx->top; i++) {
        Py_CLEAR(ctx->stack[i].obj);
        /* map_key holds a live reference only while waiting for the value */
        if (ctx->stack[i].ct == CT_MAP_VALUE) {
            Py_CLEAR(ctx->stack[i].map_key);
        }
    }

    // This captures the case where we did not push anything to the stack
    // Clear again at 0, which is safe (it just sets the pointer to NULL => no-op on second call)
    Py_CLEAR(ctx->stack[0].obj);
}