diff --git a/ext/msgpack/unpacker.c b/ext/msgpack/unpacker.c index 97eefa99..8b36307d 100644 --- a/ext/msgpack/unpacker.c +++ b/ext/msgpack/unpacker.c @@ -386,9 +386,15 @@ static inline int read_raw_body_begin(msgpack_unpacker_t* uk, int raw_type) * instead of raising StackError like every other container type. */ return PRIMITIVE_STACK_TOO_DEEP; } + size_t barrier_depth = uk->stack.depth; int raised; obj = protected_proc_call(proc, 1, &uk->self, &raised); - msgpack_unpacker_stack_pop(uk); + + /* The user proc can drive the unpacker itself (Unpacker#read, #skip, + * or a rescued error) and leave stack.depth anywhere, including 0. + * Restore it to just below the barrier we pushed instead of an + * unconditional decrement, which would underflow to SIZE_MAX. */ + uk->stack.depth = barrier_depth - 1; if (raised) { uk->last_object = rb_errinfo(); diff --git a/spec/factory_spec.rb b/spec/factory_spec.rb index d86cac4c..15b5e5d3 100644 --- a/spec/factory_spec.rb +++ b/spec/factory_spec.rb @@ -699,6 +699,48 @@ class << Symbol # recursed unbounded in C and crashed the VM (SIGSEGV) rather than raising. expect { factory.load(payload) }.to raise_error(MessagePack::StackError) end + + it 'does not corrupt the stack when a recursive unpacker leaves the depth at zero' do + # A recursive proc that rescues an inner read error, or that calls #skip, + # can drive stack.depth down to 0 before read_raw_body_begin pops its + # barrier. The unconditional pop then underflowed depth to SIZE_MAX and + # read/wrote out-of-bounds stack entries (SIGSEGV). The payloads leave no + # trailing bytes, so a fixed unpacker returns without raising. + skip if IS_JRUBY + + rescuing = MessagePack::Factory.new + rescuing.register_type(0x01, Class.new, + packer: ->(_obj, packer) { packer.write(nil) }, + unpacker: ->(u) { + begin + u.read + rescue MessagePack::MalformedFormatError, EOFError + nil + end + }, + recursive: true, + ) + + skipping = MessagePack::Factory.new + skipping.register_type(0x02, Class.new, + packer: ->(_obj, packer) { packer.write(nil) }, + unpacker: ->(u) { u.skip }, + recursive: true, + ) + + payloads = [ + [rescuing, "\xd4\x01\xc1".b], # fixext1 type=1, then an invalid byte + [rescuing, "\xd4\x01".b], # fixext1 type=1, then truncated (EOF) + [skipping, "\xd4\x02\x91\x2a".b], # fixext1 type=2, fixarray(1), 42 + ] + payloads.each do |factory, bytes| + 100.times { factory.unpack(bytes) } + end + + # The stack is intact: a fresh unpack still decodes correctly. + expect(rescuing.unpack(MessagePack.pack([1, 2, 3]))).to eq([1, 2, 3]) + expect(skipping.unpack(MessagePack.pack("ok"))).to eq("ok") + end end describe 'memsize' do