Skip to content

fix(decode): MessageLen handles ext markers instead of panicking - #391

Open
prostomarkeloff wants to merge 1 commit into
3Hren:masterfrom
prostomarkeloff:fix-messagelen-ext-markers
Open

prostomarkeloff wants to merge 1 commit into
3Hren:masterfrom
prostomarkeloff:fix-messagelen-ext-markers

Conversation

@prostomarkeloff

@prostomarkeloff prostomarkeloff commented Oct 23, 2026 •

Copy link
Copy Markdown

Found with find-dup-defs using its converge pass, which reports behavioural divergences between near-duplicate functions. It flagged MessageLen's marker-dispatch (read_one_item / read_marker_with_len) as near-duplicates whose ext arms diverge from the rest — they are todo!() while every other marker decodes a length.

MessageLen::{len_of, incremental_len} therefore panic via todo!() on any ext marker: both functions have Marker::Ext8 | Ext16 | Ext32 => todo!() and FixExt1..=FixExt16 => todo!(). ext is a normal MessagePack type (timestamps are fixext4/fixext8), so e.g. the three bytes d4 01 2a (a fixext1) panic the parser — in every build, not only with overflow checks.

MessageLen is documented as a way to find a message boundary while reading a stream, i.e. it runs on incoming bytes that may be untrusted, so this is a denial of service on malformed input rather than just a missing feature.

Fix

Route the length-carrying ext markers (ext8/ext16/ext32) through the same length decode as Str/Bin/Array/Map — MarkerLen::size() already returns 1/2/4 for them, so resumable parsing keeps working — and skip type byte + data for the fixed-size fixext* markers. For ext8/16/32 the data skip is len + 1 (the extra type byte); len < max_len <= u32::MAX, so the + 1 cannot overflow.

Tests

Added rmp/tests/repro_message_len_ext.rs: a fixext1, an ext8 inside an array, a truncated ext16 (expects Truncated), and a mixed-size round-trip built with write_ext_meta whose length matches byte-for-byte both via len_of and via chunked incremental_len. The full rmp test suite (181 unit + 4 new + 13 doctests) passes.

Disclosure: I used an AI-assisted tool while investigating and preparing this change; I have reviewed the fix and tests and can explain each line.

MessageLen::read_one_item / read_marker_with_len hit todo!() for every
ext marker (fixext1..16, ext8/16/32). Since MessageLen parses incoming,
possibly untrusted stream bytes to find a message boundary, three bytes
(e.g. a fixext1) panic len_of / incremental_len. Handle ext like the
other length-prefixed types; fixext* skip the type byte plus fixed data.
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.

1 participant