From 1d80e64c694e3e670126230a92977eee83950636 Mon Sep 17 00:00:00 2001 From: aharshit123456 Date: Sun, 26 Jul 2026 14:08:37 +0530 Subject: [PATCH] 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) + } +}