From 1d80e64c694e3e670126230a92977eee83950636 Mon Sep 17 00:00:00 2001 From: aharshit123456 Date: Sun, 26 Jul 2026 14:08:37 +0530 Subject: [PATCH 1/4] fix: verify Schnorr signatures on incoming geohash Nostr events Geohash chat (kind 20000) and presence (kind 20001) events, and location notes (kind 1), were rendered/processed without ever calling NostrEvent.isValidSignature(). A relay-side attacker (or anyone able to publish to a connected relay) could forge an event claiming any victim's pubkey and have it accepted as genuine in GeohashMessageHandler, NostrClient.handleGeohashMessage, and LocationNotesManager.handleEvent. Add a signature check at all three ingestion points, dropping any event whose signature doesn't verify against its claimed pubkey before further processing (dedup, PoW check, participant tracking, or UI rendering). Adds GeohashMessageHandlerSignatureTest, which fails against the old code (forged event accepted and rendered) and passes against the fix, while confirming genuinely-signed events are unaffected. Fixes #733 Co-Authored-By: Claude Sonnet 5 --- .../android/nostr/GeohashMessageHandler.kt | 4 + .../android/nostr/LocationNotesManager.kt | 5 + .../com/bitchat/android/nostr/NostrClient.kt | 5 + .../GeohashMessageHandlerSignatureTest.kt | 95 +++++++++++++++++++ 4 files changed, 109 insertions(+) create mode 100644 app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt 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 1c4896f3..67599668 100644 --- a/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt +++ b/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt @@ -48,6 +48,10 @@ class GeohashMessageHandler( 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 + } // PoW validation (if enabled) - apply to chat messages primarily if (event.kind == NostrKind.EPHEMERAL_EVENT) { diff --git a/app/src/main/java/com/bitchat/android/nostr/LocationNotesManager.kt b/app/src/main/java/com/bitchat/android/nostr/LocationNotesManager.kt index dc1a8e85..a8b907fd 100644 --- a/app/src/main/java/com/bitchat/android/nostr/LocationNotesManager.kt +++ b/app/src/main/java/com/bitchat/android/nostr/LocationNotesManager.kt @@ -361,6 +361,11 @@ class LocationNotesManager private constructor() { Log.v(TAG, "Ignoring non-text-note event: kind=${event.kind}") return } + + if (!event.isValidSignature()) { + Log.w(TAG, "Rejecting note ${event.id.take(8)}... with invalid signature") + return + } // Check for geohash tag val geohashTag = event.tags.firstOrNull { it.size >= 2 && it[0] == "g" } diff --git a/app/src/main/java/com/bitchat/android/nostr/NostrClient.kt b/app/src/main/java/com/bitchat/android/nostr/NostrClient.kt index a4803157..a3471662 100644 --- a/app/src/main/java/com/bitchat/android/nostr/NostrClient.kt +++ b/app/src/main/java/com/bitchat/android/nostr/NostrClient.kt @@ -282,6 +282,11 @@ class NostrClient private constructor(private val context: Context) { handler: (content: String, senderPubkey: String, nickname: String?, timestamp: Int) -> Unit ) { try { + if (!event.isValidSignature()) { + Log.w(TAG, "🚫 Rejecting geohash event ${event.id.take(8)}... with invalid signature") + return + } + // Check Proof of Work validation for incoming geohash events val powSettings = PoWPreferenceManager.getCurrentSettings() if (powSettings.enabled && powSettings.difficulty > 0) { diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt new file mode 100644 index 00000000..2f467dd9 --- /dev/null +++ b/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt @@ -0,0 +1,95 @@ +package com.bitchat.android.nostr + +import android.app.Application +import androidx.test.core.app.ApplicationProvider +import com.bitchat.android.ui.ChatState +import com.bitchat.android.ui.DataManager +import com.bitchat.android.ui.MessageManager +import kotlinx.coroutines.ExperimentalCoroutinesApi +import kotlinx.coroutines.test.TestScope +import kotlinx.coroutines.test.UnconfinedTestDispatcher +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNull +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +/** + * Regression coverage for GH-733: geohash events (kind 20000/20001) were accepted from the + * relay without Schnorr signature verification, allowing full pubkey impersonation. + */ +@RunWith(RobolectricTestRunner::class) +class GeohashMessageHandlerSignatureTest { + + private val application: Application = ApplicationProvider.getApplicationContext() + + @OptIn(ExperimentalCoroutinesApi::class) + private val testDispatcher = UnconfinedTestDispatcher() + private val testScope = TestScope(testDispatcher) + + private lateinit var chatState: ChatState + private lateinit var dataManager: DataManager + private lateinit var messageManager: MessageManager + private lateinit var repo: GeohashRepository + private lateinit var handler: GeohashMessageHandler + + private val geohash = "u4pruy" + + @Before + fun setup() { + chatState = ChatState(scope = testScope) + dataManager = DataManager(context = application) + messageManager = MessageManager(state = chatState) + repo = GeohashRepository(application, chatState, dataManager) + handler = GeohashMessageHandler(application, chatState, messageManager, repo, testScope, dataManager) + } + + private fun buildSignedEvent(identity: NostrIdentity, content: String): NostrEvent { + val unsigned = NostrEvent( + pubkey = identity.publicKeyHex, + createdAt = (System.currentTimeMillis() / 1000).toInt(), + kind = NostrKind.EPHEMERAL_EVENT, + tags = listOf(listOf("g", geohash)), + content = content + ) + return unsigned.sign(identity.privateKeyHex) + } + + @Test + fun onEvent_acceptsGenuinelySignedEvent() { + val victim = NostrIdentity.generate() + val genuine = buildSignedEvent(victim, "hello from the real victim") + + handler.onEvent(genuine, geohash) + + val stored = chatState.getChannelMessagesValue()["geo:$geohash"] + assertEquals(1, stored?.size) + assertEquals("hello from the real victim", stored?.first()?.content) + } + + @Test + fun onEvent_rejectsEventWithForgedSignature() { + val victim = NostrIdentity.generate() + val attacker = NostrIdentity.generate() + + // Attacker claims the victim's pubkey but signs with their own key - a forged event. + val forged = buildSignedEvent(victim, "impersonated message").copy( + sig = NostrEvent( + pubkey = attacker.publicKeyHex, + createdAt = (System.currentTimeMillis() / 1000).toInt(), + kind = NostrKind.EPHEMERAL_EVENT, + tags = listOf(listOf("g", geohash)), + content = "impersonated message" + ).sign(attacker.privateKeyHex).sig + ) + + // Sanity check: this event is indeed invalid before we even touch the handler. + assertEquals(false, forged.isValidSignature()) + + handler.onEvent(forged, geohash) + + val stored = chatState.getChannelMessagesValue()["geo:$geohash"] + assertNull("Forged event must not be rendered as a legitimate message", stored) + } +} From 57a7d3d955bae6ae3bad97e6abf71e8625fc0447 Mon Sep 17 00:00:00 2001 From: aharshit123456 Date: Sun, 26 Jul 2026 14:48:27 +0530 Subject: [PATCH 2/4] 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) + } } From 7e2248073209d17d6b014e21091d2e2d3fe40f41 Mon Sep 17 00:00:00 2001 From: a1denvalu3 Date: Thu, 30 Jul 2026 19:30:20 +0200 Subject: [PATCH 3/4] fix: verify geohash signatures off main thread --- .../bitchat/android/nostr/GeohashMessageHandler.kt | 11 +++++++++-- .../nostr/GeohashMessageHandlerSignatureTest.kt | 10 +++++++++- 2 files changed, 18 insertions(+), 3 deletions(-) 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 19e5a369..5d80e22d 100644 --- a/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt +++ b/app/src/main/java/com/bitchat/android/nostr/GeohashMessageHandler.kt @@ -5,8 +5,11 @@ import android.util.Log import com.bitchat.android.model.BitchatMessage import com.bitchat.android.ui.ChatState import com.bitchat.android.ui.MessageManager +import kotlinx.coroutines.CoroutineDispatcher import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch +import kotlinx.coroutines.withContext import java.util.Date /** @@ -21,7 +24,8 @@ class GeohashMessageHandler( private val messageManager: MessageManager, private val repo: GeohashRepository, private val scope: CoroutineScope, - private val dataManager: com.bitchat.android.ui.DataManager + private val dataManager: com.bitchat.android.ui.DataManager, + private val signatureVerificationDispatcher: CoroutineDispatcher = Dispatchers.Default ) { companion object { private const val TAG = "GeohashMessageHandler" } @@ -47,7 +51,10 @@ 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 (!event.isValidSignature()) { + val hasValidSignature = withContext(signatureVerificationDispatcher) { + event.isValidSignature() + } + if (!hasValidSignature) { Log.w(TAG, "Rejecting geohash event ${event.id.take(8)}... with invalid signature") return@launch } 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 e0e2c8cd..d9a557d8 100644 --- a/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/nostr/GeohashMessageHandlerSignatureTest.kt @@ -42,7 +42,15 @@ class GeohashMessageHandlerSignatureTest { dataManager = DataManager(context = application) messageManager = MessageManager(state = chatState) repo = GeohashRepository(application, chatState, dataManager) - handler = GeohashMessageHandler(application, chatState, messageManager, repo, testScope, dataManager) + handler = GeohashMessageHandler( + application, + chatState, + messageManager, + repo, + testScope, + dataManager, + testDispatcher + ) } private fun buildSignedEvent(identity: NostrIdentity, content: String): NostrEvent { From e07e3c91186877f3d59414afd8cf1baac6413e4a Mon Sep 17 00:00:00 2001 From: a1denvalu3 Date: Thu, 30 Jul 2026 19:57:07 +0200 Subject: [PATCH 4/4] test: sign background geohash events --- .../NostrBackgroundEventProcessorTest.kt | 22 +++++++++++-------- 1 file changed, 13 insertions(+), 9 deletions(-) diff --git a/app/src/test/kotlin/com/bitchat/android/nostr/NostrBackgroundEventProcessorTest.kt b/app/src/test/kotlin/com/bitchat/android/nostr/NostrBackgroundEventProcessorTest.kt index d7fffc2c..152d4ad0 100644 --- a/app/src/test/kotlin/com/bitchat/android/nostr/NostrBackgroundEventProcessorTest.kt +++ b/app/src/test/kotlin/com/bitchat/android/nostr/NostrBackgroundEventProcessorTest.kt @@ -36,17 +36,21 @@ class NostrBackgroundEventProcessorTest { fun `cold start processes more events than the removed handoff queue capacity`() = runBlocking { val application = ApplicationProvider.getApplicationContext() val processor = NostrBackgroundEventProcessor(application, scope) + val identity = NostrIdentity.generate() + val expectedIds = mutableSetOf() repeat(300) { index -> + val event = NostrEvent( + pubkey = identity.publicKeyHex, + createdAt = index + 1, + kind = NostrKind.EPHEMERAL_EVENT, + tags = listOf(listOf("g", "u4pruy")), + content = "message-$index" + ).sign(identity.privateKeyHex) + expectedIds += event.id + processor.onGeohashMessage( - event = NostrEvent( - id = "cold-start-$index", - pubkey = index.toString(16).padStart(64, '0'), - createdAt = 1, - kind = NostrKind.EPHEMERAL_EVENT, - tags = listOf(listOf("g", "u4pruy")), - content = "message-$index" - ), + event = event, geohash = "u4pruy" ) } @@ -58,7 +62,7 @@ class NostrBackgroundEventProcessorTest { } assertEquals( - (0 until 300).map { "cold-start-$it" }.toSet(), + expectedIds, AppStateStore.channelMessages.value["geo:u4pruy"].orEmpty().map { it.id }.toSet() ) }