fix: validate ext frame bounds before byte-slice decode#60
fix: validate ext frame bounds before byte-slice decode#60hyp3rd wants to merge 2 commits intoshamaton:mainfrom
Conversation
Reject truncated fixext/ext frames before dispatching to extension decoders in the byte-slice decode path. This prevents malformed timestamp ext values from reaching unsafe reads and panicking during Unmarshal. Also reuse the same validation in ext skipping paths, harden the built-in time decoder to return ErrTooShortBytes on short input, and add regression coverage for Unmarshal*, custom ext dispatch, and timestamp short-buffer cases.
There was a problem hiding this comment.
Pull request overview
Fixes a panic when decoding truncated MessagePack extension (ext) frames—especially Timestamp ext values—by validating ext frame bounds before dispatching to ext decoders, and by hardening the built-in time decoders to return def.ErrTooShortBytes on short input.
Changes:
- Add ext-frame length validation in the byte-slice decoder before ext decoder dispatch, and reuse it when skipping ext values.
- Harden Timestamp decoding (
IsType/AsValueand stream decodeToValue) to safely handle short buffers. - Add regression tests covering truncated ext inputs across multiple unmarshal entrypoints and decoding paths.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| unmarshal_ext_test.go | Regression test ensuring truncated Timestamp ext inputs return ErrTooShortBytes across Unmarshal variants. |
| time/decode.go | Adds safe read helpers and makes Timestamp IsType/AsValue robust to short input. |
| time/decode_test.go | Adds tests for Timestamp decoder short-input behavior (IsType and AsValue). |
| time/decode_stream.go | Adds length checks in stream Timestamp decoding to avoid panics on short payloads. |
| time/decode_stream_test.go | Adds tests asserting stream Timestamp decoding returns ErrTooShortBytes on short payloads. |
| internal/decoding/ext.go | Introduces shared ext-frame end-offset validation used to reject truncated frames early. |
| internal/decoding/ext_test.go | Adds tests ensuring truncated ext frames are rejected before custom ext decoders are invoked. |
| internal/decoding/interface.go | Validates ext frame bounds before attempting ext decoder dispatch for interface{} decoding. |
| internal/decoding/interface_test.go | Adjusts/extents tests for truncated ext behavior and expected errors. |
| internal/decoding/struct.go | Validates ext frame bounds before ext decoder dispatch for struct decoding; uses shared ext skipping logic. |
| internal/decoding/struct_test.go | Adds coverage for skipping/offset logic when encountering truncated ext frames. |
| msgpack_test.go | Updates expectation for truncated Ext32 input to “too short bytes”. |
| README.md | Formatting-only changes (adds blank lines / alters indentation in Quick Start snippet). |
| go.mod | Updates declared Go version. |
| .github/workflows/test.yml | Expands Go version matrix to include 1.26. |
Comments suppressed due to low confidence (1)
go.mod:4
- The
godirective is set to1.26.1, which both bumps the minimum supported Go version and (depending on Go toolchain parsing) may be an invalid format for thegodirective (typicallymajor.minor, with patch versions specified via atoolchaindirective if needed). If the intent is to require Go 1.26.x, consider usinggo 1.26(or1.26.0) and, if you need a specific patch, addtoolchain go1.26.1; otherwise CI/builds on older versions will fail to parse/build this module.
module github.com/shamaton/msgpack/v3
go 1.26.1
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
We still use v2. Is it possible to have fix for v2? |
Sure, I can submit a different PR |
|
…k dependency Disable Marshal and Unmarshal in MsgpackSerializer by converting them into stubs that return errors. This addresses a security concern in the upstream shamaton/msgpack library (ref: shamaton/msgpack#60). The type is marked deprecated and will be removed in a future release. - Remove github.com/shamaton/msgpack/v3 from go.mod - Bump github.com/hyp3rd/ewrap from v1.3.8 to v1.3.9
…k dependency Disable Marshal and Unmarshal in MsgpackSerializer by converting them into stubs that return errors. This addresses a security concern in the upstream shamaton/msgpack library (ref: shamaton/msgpack#60). The type is marked deprecated and will be removed in a future release. - Remove github.com/shamaton/msgpack/v3 from go.mod - Bump github.com/hyp3rd/ewrap from v1.3.8 to v1.3.9
Summary
This fixes a panic in the byte-slice decode path when
Unmarshalreceives truncated ext data, especially truncatedfixexttimestamp payloads.Before this change, the decoder could dispatch malformed ext values to the built-in timestamp decoder before the full frame length was validated. That allowed short input to reach unsafe reads and panic instead of returning a normal decode error.
What changed
fixext1/2/4/8/16andext8/16/32IsTypereturnsfalseon short inputAsValuereturnsdef.ErrTooShortByteson incomplete ext payloadsUnmarshalUnmarshalAsArrayUnmarshalAsMapResult
Malformed truncated ext input now fails with
def.ErrTooShortBytesinstead of panicking.This keeps the public decode and ext-decoder APIs unchanged.
Testing
go test ./...