From 57a7d3d955bae6ae3bad97e6abf71e8625fc0447 Mon Sep 17 00:00:00 2001 From: aharshit123456 Date: Sun, 26 Jul 2026 14:48:27 +0530 Subject: [PATCH] fix: verify signature before dedup in GeohashMessageHandler MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Per Codex review on PR #743: the previous ordering called dedupe(event.id) before isValidSignature(), so a forged event (bad signature, but a content-derived id matching a legitimate event) would get its id marked as "seen" and dropped — permanently poisoning the dedup cache. When the genuine, validly-signed copy of the same event later arrived (e.g. via a different relay), it was then silently discarded too, as an apparent duplicate. Move the signature check ahead of dedupe() so only genuinely-authenticated event ids are ever cached, matching the ordering already used correctly in NostrClient.handleGeohashMessage() and LocationNotesManager.handleEvent(). Adds a regression test that reproduces the exact scenario: a forged event is rejected first, then a genuine event with the same id must still be rendered (previously failed against the pre-fix ordering). Co-Authored-By: Claude Sonnet 5 --- .../android/nostr/GeohashMessageHandler.kt | 2 +- .../GeohashMessageHandlerSignatureTest.kt | 34 +++++++++++++++++++ 2 files changed, 35 insertions(+), 1 deletion(-) diff --git a/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt b/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt index 67599668..19e5a369 100644 --- a/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt +++ b/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt @@ -47,11 +47,11 @@ class GeohashMessageHandler( if (event.kind != NostrKind.EPHEMERAL_EVENT && event.kind != NostrKind.GEOHASH_PRESENCE) return@launch val tagGeo = event.tags.firstOrNull { it.size >= 2 && it[0] == "g" }?.getOrNull(1) if (tagGeo == null || !tagGeo.equals(subscribedGeohash, true)) return@launch - if (dedupe(event.id)) return@launch if (!event.isValidSignature()) { Log.w(TAG, "Rejecting geohash event ${event.id.take(8)}... with invalid signature") return@launch } + if (dedupe(event.id)) return@launch // PoW validation (if enabled) - apply to chat messages primarily if (event.kind == NostrKind.EPHEMERAL_EVENT) { diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt index 2f467dd9..e0e2c8cd 100644 --- a/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt @@ -92,4 +92,38 @@ class GeohashMessageHandlerSignatureTest { val stored = chatState.getChannelMessagesValue()["geo:$geohash"] assertNull("Forged event must not be rendered as a legitimate message", stored) } + + @Test + fun onEvent_stillAcceptsGenuineEventAfterForgedCopyWithSameIdWasRejected() { + val victim = NostrIdentity.generate() + val attacker = NostrIdentity.generate() + val genuine = buildSignedEvent(victim, "hello from the real victim") + + // A forged event carrying the SAME id as the genuine one (ids are a content hash, + // independent of the signature), but signed by an attacker - relays can deliver this + // before the genuine copy arrives from another relay. + val forgedWithSameId = genuine.copy( + sig = NostrEvent( + pubkey = attacker.publicKeyHex, + createdAt = genuine.createdAt, + kind = NostrKind.EPHEMERAL_EVENT, + tags = listOf(listOf("g", geohash)), + content = "hello from the real victim" + ).sign(attacker.privateKeyHex).sig + ) + assertEquals(genuine.id, forgedWithSameId.id) + assertEquals(false, forgedWithSameId.isValidSignature()) + + // Forged copy arrives first and must be rejected without poisoning the dedup cache. + handler.onEvent(forgedWithSameId, geohash) + assertNull(chatState.getChannelMessagesValue()["geo:$geohash"]) + + // The genuine copy (same id, valid signature) arrives afterwards from another relay - + // it must still be rendered, not silently dropped as a "duplicate". + handler.onEvent(genuine, geohash) + + val stored = chatState.getChannelMessagesValue()["geo:$geohash"] + assertEquals(1, stored?.size) + assertEquals("hello from the real victim", stored?.first()?.content) + } }