Skip to content

OwnedView lends a fabricated 'static buffer to arbitrary MessageView impls with no parametricity contract #376

Description

@iainmcgin

Summary

OwnedView::<V>::decode / decode_with_options (buffa/src/view.rs) transmute the
Bytes slice to &'static [u8] and pass it to V::decode_view, where the only bound is
V: MessageView<'static>. MessageView is a safe, unsealed trait and nothing requires V
to be parametric in the buffer lifetime. OwnedView's Debug, Clone, PartialEq,
Serialize, Drop and to_owned_message impls then call V's own methods on the
un-narrowed view (still typed 'static) rather than on a reborrow()ed V::View<'_>.

Why it is unsound

A hand-written, non-parametric impl MessageView<'static> for MyView can — inside
decode_view, or inside its own Debug/PartialEq/Serialize/Drop — copy a
&'static str / &'static [u8] borrowed from the buffer into longer-lived storage
(a static, a channel, a returned value). When the OwnedView drops its Bytes, that
reference dangles: a use-after-free reachable from 100% safe downstream code (confirmed
under Miri with a Debug impl that stashes self.name).

The # Safety argument on OwnedView (Bytes is stable and immutable; view dropped before
bytes) is correct as far as it goes but silently relies on V being lifetime-parametric,
which is neither documented nor enforced.

Reachability / severity

Low. Every codegen-emitted FooView<'a> is parametric and cannot capture the buffer;
no wire input reaches this. Only bespoke hand-written view types (a supported pattern per
the guide) are exposed. This is the input-side sibling of GHSA-9pwq-gcrx-wghh /
CVE-2026-55406, whose fix (#154, removing Deref in favour of reborrow) closed the
output-side leak but left the constructor contract implicit.

Suggested direction

  • Make the requirement a checked contract at the constructor: an unsafe marker trait
    (or make hand-written MessageView impls unsafe to write, or seal MessageView to
    codegen output) documenting that decode_view and the view's own trait impls must not
    let the buffer lifetime escape Self.
  • Defence in depth: route Debug/Clone/PartialEq/Serialize/to_owned_message
    through reborrow() (bounded on V: ViewReborrow) so V's methods only ever see
    &'b V::View<'b>. Drop and decode_view itself cannot be narrowed this way, which is
    why the contract is the primary fix.
  • A Miri regression test with a deliberately capturing hand-written view.

Credit to HackerOne researcher waynezinn for discovering and reporting this issue through
Anthropic's security bug bounty program. Filed here for public tracking as hardening work.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions