Audit LOW: empty offer, attachment id, QR binding, skipped-key order

Same empty-offer and attachment-id rules as iOS. The QR scanner now
binds a scanned code by its fnid rather than the handle even on the home
relay — the fnid certifies itself, a handle is a name the relay maps,
so resolving by handle handed the relay the choice of who you scanned.
And a skipped message key is consumed only after its tag verifies: it
was defused today by the caller discarding the session, but any future
in-memory ratchet cache would have turned it into real message loss.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
This commit is contained in:
Paul Hitt
2026-08-29 09:13:25 -04:00
co-authored by Claude Opus 4.8
parent cbe2292d3c
commit 130edd7019
4 changed files with 44 additions and 9 deletions
@@ -344,12 +344,25 @@ class Conversations(
sendPlaintext(toUserId, Content.callSignal(signal))
/** Download and open a blob a message referred to. */
/**
* 64 lowercase hex, and nothing else — the shape the relay mints and the
* only shape it will serve (audit LOW).
*
* 🔴 The id arrives from a PEER and is interpolated into a URL path. The
* relay validates it before it reaches a filesystem; the client did not,
* so a crafted id could reshape the victim's own authenticated GET.
*/
private fun isValidAttachmentToken(id: String): Boolean =
id.length == 64 && id.all { it in '0'..'9' || it in 'a'..'f' }
fun fetchAttachment(ref: Attachment.Ref): ByteArray =
// The size came through the ratchet, so the relay cannot inflate it —
// which is what makes it usable as a ceiling (audit M5).
Attachment.open(
relay.downloadAttachment(
ref.id,
ref.id.also {
require(isValidAttachmentToken(it)) { "malformed attachment id" }
},
maxBytes = if (ref.bytes > 0) minOf(ref.bytes + 4096L,
RelayClient.HARD_ATTACHMENT_CAP)
else RelayClient.HARD_ATTACHMENT_CAP),
@@ -162,6 +162,14 @@ class Trust(
)
return IdentityTrust.PINNED
}
// 🔴 **An empty offer is not agreement (audit LOW).** `all {}` on an
// empty list is vacuously true, so a directory answering with NO keys
// for someone already pinned read as "unchanged" — the strongest
// verdict — when it had said nothing at all. It is not evidence of a
// swap either, so it reports UNKNOWN: nothing to compare, nothing to
// send to. The first-contact branch above already guards this case;
// this is the same rule once keys exist.
if (encoded.isEmpty()) return IdentityTrust.UNKNOWN
// ⚠️ EVERY offered key must already be pinned — not "any". A relay
// adding one key of its own alongside the real ones would otherwise
// pass, and that is precisely the attack: the victim's real devices