Malformed MLS Welcome aborts the process: unhandled unwraps in GroupV1Convo::new_from_welcome under panic = "abort"

[osmaczko]

A peer that can open a direct conversation with a client can instead crash its whole process by sending a single malformed Welcome. Opening a conversation already delivers a Welcome to the target's inbox; a Welcome that is structurally valid but not decryptable by the target aborts the process instead of being rejected. Under the release profile's panic = "abort" (Cargo.toml:55) this is a full process abort, not a recoverable error.

Root cause

GroupV1Convo::new_from_welcome (core/conversations/src/conversation/group_v1.rs:75,77,79) unwraps three fallible OpenMLS calls on the peer-supplied Welcome:

let mls_group =
    StagedWelcome::build_from_welcome(&cx.mls_provider, &Self::mls_join_config(), welcome)
        .unwrap()
        .build()
        .unwrap()
        .into_group(&cx.mls_provider)
        .unwrap();

It is reached from InboxV2::handle_frame -> handle_heavy_invite -> new_from_welcome (core/conversations/src/inbox_v2.rs:190) for every InviteType::GroupV1, i.e. every DirectV1 first-contact. The steps before it are guarded (MlsMessageIn::tls_deserialize_bytes(...)?, and msg_in.extract() with an else-branch), so a Welcome that is structurally valid but cannot be processed by the recipient (addressed to a KeyPackage this client never published, ciphersuite mismatch, or corrupted encrypted secrets) passes those checks, reaches build_from_welcome, which returns Err, and the .unwrap() aborts.

Scope

Only the GroupV1 Welcome/join path is affected. The ongoing application-message path (GroupV1Convo::handle_frame) already propagates errors (map_err/?), and the GroupV2 join path propagates too. The other unwraps in group_v1.rs (new at :57, add_member at :332/:336, key-package build at :175/:177) and inbox_v2.rs:223 are on outbound/local operations and are not reachable from peer input.

Impact

Remote-triggerable denial of service: crash any client at will given only the address it shares to receive conversations. Under panic = "abort" the whole process aborts; a debug build unwinds the worker thread instead and the client stops processing inbound.

Reproduce

Over the in-process transport: have client A open a DirectV1 with client B, capture the resulting Welcome frame off the bus, flip a byte inside its encrypted portion (keeping the TLS structure valid so tls_deserialize_bytes and extract() still yield a Welcome), redeliver it to B, and drive B's handle_payload. B panics in new_from_welcome. In the dev profile this unwinds and is catchable with #[should_panic].

Suggested fix

Propagate the error out of new_from_welcome instead of unwrapping (return a ChatError) and treat an unprocessable Welcome as a dropped inbox frame rather than a fatal event. Separately, reconsider panic = "abort" for a library that parses untrusted peer input, or isolate that parsing so a panic cannot take the host process down.

Verified at 6ab0d8a79f59cc3b5ed6626b072e7189fa677231.

[kivancgnlp]

Hi, I’d like to work on this. I’ll investigate the error propagation path and add regression coverage for malformed Welcome messages.