Depth-limit guard from #277 doesn't cover deserialize_option, deserialize_newtype_struct, or map-shaped enum newtype variants — reachable crash via attacker-controlled recursion
- Dominant language
- Rust
- Stars
- 1.4k
- Forks
- 162
- PR merge metrics
- No merged PRs in 30d
Description
#276/#52 (fixed by #277) added a `depth_count!` guard to `Deserializer`, but it only wraps three
call sites: `visit_seq`, `visit_map`, and the `Ext`-marker `visit_newtype_struct` branch inside
`deserialize_any`. Three other recursion-capable forwarding points were never covered and bypass
the guard (and `set_max_depth`) entirely:
1. `deserialize_option`'s `visitor.visit_some(self)`
2. `deserialize_newtype_struct`'s `visitor.visit_newtype_struct(self)` (both the general branch
and the `MSGPACK_EXT_STRUCT_NAME` branch)
3. `VariantAccess::newtype_variant_seed`'s `seed.deserialize(self.de)` (the map-as-single-variant
enum path)
For a type shaped as `Option>` or `Wrap(Box)` (a newtype struct wrapping itself),
a stack overflow (process abort) is reachable with essentially no recursion-driving input —
`Option>` overflows on a single non-nil byte; the newtype-struct case overflows on an
**empty buffer**. The enum-variant path does need attacker-controlled input length (a chain of
`{"Node": ...}` maps), but that's still just a few hundred bytes at the default depth of 1024.
`set_max_depth` gives no protection against any of these three — the guard is never consulted on
these paths, so a caller who explicitly configured a lower limit still crashes.
This is a crash-only bug — no memory corruption, no data exposure, just a reachable panic/abort
via `rmp_serde::from_slice`/`Deserializer` on attacker-supplied bytes. Worth fixing regardless of
where you'd land it on a severity scale; flagging that framing up front rather than dressing it up.
### Reproduction (rmp-serde 1.3.1, and current master @ cf88001)
```rust
use serde::Deserialize;
#[derive(Deserialize)]
struct ONode(Option>);
fn main() {
// A single byte -- 0x00 is a fixint 0, NOT the 0xc0 nil marker -- drives unbounded
// recursion for ONode's Option> shape. Aborts with a stack overflow.
let _: Result = rmp_serde::from_slice(&[0x00]);
}
```
```rust
use serde::Deserialize;
#[derive(Deserialize)]
struct Wrap(Box);
fn main() {
// Empty buffer. deserialize_newtype_struct never reads a byte before recursing.
let _: Result = rmp_serde::from_slice(&[]);
}
```
```rust
use serde::Deserialize;
#[derive(Deserialize)]
enum EList { Leaf, Node(Box) }
fn main() {
let mut data = Vec::new();
for _ in 0..200_000 {
data.extend_from_slice(&[0x81, 0xa4, b'N', b'o', b'd', b'e']); // {"Node": ...}
}
data.extend_from_slice(&[0xa4, b'L', b'e', b'a', b'f']);
let _: Result = rmp_serde::from_slice(&data);
}
```
### Fix
PR incoming: extends `depth_count!` to all three sites, with an explicit reborrow (`&mut *self`)
where the callee is generic over `D: Deserializer<'de>` — Rust doesn't auto-reborrow through a
generic parameter the way it does for a concretely-typed `&mut T` parameter, so a bare
`self`/`self.de` would move rather than reborrow there and fail to compile.
---
Discovered by the [rust-in-peace](https://github.com/scadastrangelove/rust-in-peace/) security pipeline.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start at Deserializer's existing depth_count! wrappers and inspect deserialize_option, deserialize_newtype_struct, and VariantAccess::newtype_variant_seed. Run the supplied recursive Option, newtype-struct, and map-shaped enum reproductions; done means all three paths honor set_max_depth without reaching stack overflow.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- security
- Issue type
- Bug
- Difficulty
- 3/5
- Estimated time
- 1-2 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 35/100