Skip to content

fix: validate ext frame bounds before byte-slice decode#60

Open
hyp3rd wants to merge 2 commits intoshamaton:mainfrom
hyp3rd:fix/GO-2026-4513
Open

fix: validate ext frame bounds before byte-slice decode#60
hyp3rd wants to merge 2 commits intoshamaton:mainfrom
hyp3rd:fix/GO-2026-4513

Conversation

@hyp3rd
Copy link
Copy Markdown

@hyp3rd hyp3rd commented Mar 23, 2026

Summary

This fixes a panic in the byte-slice decode path when Unmarshal receives truncated ext data, especially truncated fixext timestamp 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

  • validate full ext frame length in the byte-slice decoder before ext decoder dispatch
  • cover fixext1/2/4/8/16 and ext8/16/32
  • reuse the same validation when skipping ext values so truncated frames cannot advance offsets silently
  • harden the built-in time decoder so:
    • IsType returns false on short input
    • AsValue returns def.ErrTooShortBytes on incomplete ext payloads
  • add regression tests for:
    • Unmarshal
    • UnmarshalAsArray
    • UnmarshalAsMap
    • truncated ext dispatch before custom decoders run
    • ext offset skipping
    • timestamp decode short-buffer behavior

Result

Malformed truncated ext input now fails with def.ErrTooShortBytes instead of panicking.

This keeps the public decode and ext-decoder APIs unchanged.

Testing

  • go test ./...

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.
Copilot AI review requested due to automatic review settings March 23, 2026 10:23
Copy link
Copy Markdown

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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/AsValue and stream decode ToValue) 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 go directive is set to 1.26.1, which both bumps the minimum supported Go version and (depending on Go toolchain parsing) may be an invalid format for the go directive (typically major.minor, with patch versions specified via a toolchain directive if needed). If the intent is to require Go 1.26.x, consider using go 1.26 (or 1.26.0) and, if you need a specific patch, add toolchain 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.

@cxyzhang0
Copy link
Copy Markdown

We still use v2. Is it possible to have fix for v2?

@hyp3rd
Copy link
Copy Markdown
Author

hyp3rd commented Mar 25, 2026

We still use v2. Is it possible to have fix for v2?

Sure, I can submit a different PR

@cxyzhang0
Copy link
Copy Markdown

We still use v2. Is it possible to have fix for v2?

Sure, I can submit a different PR

Great. Also is there a reason why ithe go version bump to 1.26.1?

hyp3rd added a commit to hyp3rd/hypercache that referenced this pull request Apr 4, 2026
…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
hyp3rd added a commit to hyp3rd/hypercache that referenced this pull request Apr 4, 2026
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants