Skip to content

Nickname save race: public→private mid-edit can still leak plaintext member_info #318

@sanity

Description

@sanity

Problem (surfaced during PR #317 review)

PR #317 fixed the #299 leak where a private-room nickname edit could publish SealedBytes::public if the room secret hadn't arrived yet. A narrower follow-up race remains:

  1. User opens the member-info modal in a public room and starts editing their nickname.
  2. NicknameField::save_changes runs synchronously: reads is_private = false, no secret, calls seal_for_room(false, None, ...) → returns Some(SealedBytes::public(...)) (correct for a public room).
  3. The closure builds the delta and schedules a crate::util::defer(...) (setTimeout(0)) closure to apply it.
  4. Between steps 3 and 4, a network message arrives changing the room to private mode and ROOMS.with_mut applies that config update.
  5. Our deferred closure fires — ROOMS.with_mut now sees a private room, but our pre-built delta carries a plaintext member_info entry. apply_delta accepts it (common/src/room_state/member_info.rs validates declared length and signature only — no privacy check). mark_needs_sync then publishes the plaintext into the now-private room.

Window is tight (single event-loop tick, typically ~4ms) but real.

Why this only affects nickname (not room name / description)

ui/src/components/room_list/room_name_field.rs and edit_room_modal.rs::RoomDescriptionField publish into Configuration. common/src/room_state/configuration.rs::apply_delta (around L112–115) rejects a SealedBytes::Public display-metadata write to a private room, so the bad delta would fail to apply locally and mark_needs_sync wouldn't fire. The leak is bounded to member_info for now.

Fix direction

Move the seal+delta build into the deferred closure for NicknameField, so is_private / current_secret_opt are read at the same moment as apply_delta. With both reads in the same ROOMS.with_mut block the race window collapses to zero. If seal_for_room returns None inside the deferred block, surface the revert via temp_nickname.set(...) from inside the closure (a deferred-context signal write is safe).

A stronger defense-in-depth fix would be to add a privacy guard to common/src/room_state/member_info.rs::apply_delta mirroring configuration.rs: reject a SealedBytes::Public preferred_nickname write when the room is PrivacyMode::Private. That would also force a delegate/contract WASM migration but would close this race at the contract level for all current and future UI surfaces.

Scope notes

[AI-assisted - Claude]

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions