nik/bitchat-android
1
0
Fork 0
mirror of https://github.com/permissionlesstech/bitchat-android.git synced 2026-08-22 07:06:05 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix: verify signature before dedup in GeohashMessageHandler

Browse Source 
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 <noreply@anthropic.com>
This commit is contained in:
aharshit123456 2026-07-26 14:48:27 +05:30
parent 1d80e64c69
commit 57a7d3d955
2 changed files with 35 additions and 1 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt
View File

@ -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) {

34
app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt
View File

@ -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)
}
}