Skip to content

channels: a live-head rewrite after a rotation reopens a boundary already graded credential-safe #13490

Description

@chenmingwei23

What is the problem?

A length rotation grades the boundary it creates, then a different writer replaces the
live head
and the graded boundary is no longer the boundary the reader gets.

_rotate_on_length seals chunks up to a boundary it has checked with
joins_to_a_credential, and keeps the remainder live. That check is a statement about the
text present at that moment. Two paths then overwrite the live text without re-checking:

  • Discord's table-card branch assigns self._delivery_text after a rotation has already
    sealed a frame, so the new live head is not the tail that was graded.
  • apply_options_cap expansion rewrites the live segment on the same footing.

A credential whose first half ended the last sealed message and whose second half opens
the replacement head is therefore readable on screen, even though every boundary the
rotation itself made was refused or cleared.

Why this issue matters to the user

It is the same visible outcome as a severed cut: the reader's client puts two messages one
under the other and shows an intact key that neither message matched on its own. Each
message is redacted alone, so nothing in the per-message scan can see it. No markup is
needed -- a plain unmarked key split this way leaks.

The window is narrower than the rotation cut (it needs a head rewrite to land right after
a seal, with the key's remainder at the start of the new head), which is why it is filed
separately rather than held against the rotation fix.

How our fix would solve it

The boundary must be graded by whoever creates it, not only by the rotation. Either:

  1. grade at the seal -- the last sealed frame is known there, so _seal_current can check
    the pair whatever wrote the head; or
  2. make the head rewriters go through one accessor that re-grades against the last sealed
    frame and withholds rather than sends.

(1) is the smaller surface and covers writers not yet written; (2) keeps the cost off the
seal path. Either way the failure direction is to withhold: the withheld text rides the
next delivery, where the whole segment is redacted as one string.

What tests we did

The gap is pinned today as a strict xfail in test_discord.py
(TestRotationSeamCredentialSafety::test_a_head_rewritten_after_grading_reopens_the_seam):
it rotates on a body whose graded boundary is safe, replaces the live head with the key's
other half, and asserts the screen carries no credential. It fails, so the marker holds.
Removing that marker is part of the fix.

Any other suggestions on the work

The boundaries the rotation itself makes are graded on both channels, including the
table-card split, so this is the remaining hole in the same class rather than a new one.
Telegram has no equivalent head-rewriting branch today, but the accessor in option (2)
would be shared, so it is worth deciding once.

Activity

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

Metadata

Metadata

Assignees

Labels

area: channelsSlack, Discord, WeCom, notification routingclaimedAn automated fix is in progresssecurityHardening, trust boundary, or sandbox escape

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions