diff --git a/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt b/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt index 7263abd..af3381f 100644 --- a/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt +++ b/app/src/main/kotlin/io/flamenet/messenger/app/MainActivity.kt @@ -724,12 +724,13 @@ internal fun ColumnScope.SignedIn( onFound = { raw -> scanning = false parseScannedAddress(raw)?.let { card -> - // A same-relay code that carries the handle can go through - // the ordinary username lookup; anything else fills the - // field with the address it named. - scannedPrefill = if (card.handle != null && - card.relay.equals(phase.relay, ignoreCase = true) - ) card.handle else "${card.fnid}@${card.relay}" + // 🔴 **Bind by the fnid, never the handle (audit LOW).** + // The fnid is derived from the account key and certifies + // itself; a handle is a name the RELAY maps to an account, + // so resolving a scanned code by handle handed the relay + // the choice of who you just scanned. iOS always binds the + // address; this now matches it, same relay or not. + scannedPrefill = "${'$'}{card.fnid}@${'$'}{card.relay}" addingContact = true } }, diff --git a/client/src/main/kotlin/io/flamenet/client/Conversations.kt b/client/src/main/kotlin/io/flamenet/client/Conversations.kt index f78c991..8648fb4 100644 --- a/client/src/main/kotlin/io/flamenet/client/Conversations.kt +++ b/client/src/main/kotlin/io/flamenet/client/Conversations.kt @@ -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), diff --git a/client/src/main/kotlin/io/flamenet/client/Trust.kt b/client/src/main/kotlin/io/flamenet/client/Trust.kt index ffaf289..53df7a7 100644 --- a/client/src/main/kotlin/io/flamenet/client/Trust.kt +++ b/client/src/main/kotlin/io/flamenet/client/Trust.kt @@ -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 diff --git a/engine/src/main/kotlin/io/flamenet/e2e/DoubleRatchet.kt b/engine/src/main/kotlin/io/flamenet/e2e/DoubleRatchet.kt index 8c2e8ac..3a01ac0 100644 --- a/engine/src/main/kotlin/io/flamenet/e2e/DoubleRatchet.kt +++ b/engine/src/main/kotlin/io/flamenet/e2e/DoubleRatchet.kt @@ -201,10 +201,23 @@ class DoubleRatchet private constructor( val aad = associatedData + message.header.canonicalJson() // A key held for a message that never arrived, or arrived late. + // + // 🔴 **Removed only AFTER the tag verifies (audit LOW).** Consuming it + // first meant a forged ciphertext for a stashed (dh, n) burned the key + // the real delayed message needed — the genuine message could then + // never be opened. Today the caller discards the whole session object + // on a failed decrypt, so the loss never reaches the vault; that is a + // property of the CALLER, and the moment anything caches a ratchet in + // memory it would become a real message-loss bug. iOS has always + // removed after the open for exactly this reason. val skippedKey = SkippedKey(Bytes.b64(message.header.dh), message.header.n) - skipped.remove(skippedKey)?.let { mk -> + skipped[skippedKey]?.let { mk -> val (aesKey, nonce) = aeadParams(mk) - return Aead.open(aesKey, nonce + message.ciphertext, aad) + val plaintext = Aead.open(aesKey, nonce + message.ciphertext, aad) + // LinkedHashMap keeps insertion order, so removing from it is the + // whole eviction bookkeeping — there is no separate order list. + skipped.remove(skippedKey) + return plaintext } // A new ratchet key from the far side: catch up on their old chain,