diff --git a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt index 12d34ba2d6..8398deba54 100644 --- a/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt +++ b/amethyst/src/main/java/com/vitorpamplona/amethyst/model/LocalCache.kt @@ -2075,6 +2075,12 @@ object LocalCache : ILocalCache, ICacheProvider { if (note.event != null) return false if (wasVerified || justVerify(event)) { + val expectedServicePubkey = + event.walletServicePubKey() ?: run { + Log.w("LocalCache", "NWC request ${event.id} has no `p` tag; cannot register for response.") + return false + } + note.loadEvent(event, author, emptyList()) relay?.let { @@ -2083,7 +2089,7 @@ object LocalCache : ILocalCache, ICacheProvider { zappedNote?.addZapPayment(note, null) - paymentTracker.registerRequest(event.id, zappedNote, onResponse) + paymentTracker.registerRequest(event.id, expectedServicePubkey, zappedNote, onResponse) refreshNewNoteObservers(note) @@ -2100,13 +2106,28 @@ object LocalCache : ILocalCache, ICacheProvider { ): Boolean { val requestId = event.requestId() val pending = - paymentTracker.onResponseReceived(requestId) ?: run { - Log.w( - "LocalCache", - "NWC response ${event.id} from ${event.pubKey} references request e=$requestId but no pending request is registered. " + - "The response was either delivered after timeout, the user holds a stale subscription, or the wallet service set the wrong e tag.", - ) - return false + when (val match = paymentTracker.onResponseReceived(requestId, event.pubKey)) { + is NwcPaymentTracker.MatchResult.Matched -> match.pending + is NwcPaymentTracker.MatchResult.WrongAuthor -> { + // Possible spoof: a kind-23195 event from someone other than + // the wallet service we sent the request to. The pending + // entry is left in place so the real response can still + // resolve it; we silently drop this one. + Log.w( + "LocalCache", + "Rejecting NWC response ${event.id}: expected author ${match.expected} but event was signed by ${match.actual}. " + + "This may be a spoofed reply — keeping the request pending for the legitimate wallet response.", + ) + return false + } + NwcPaymentTracker.MatchResult.NoMatch -> { + Log.w( + "LocalCache", + "NWC response ${event.id} from ${event.pubKey} references request e=$requestId but no pending request is registered. " + + "The response was either delivered after timeout, the user holds a stale subscription, or the wallet service set the wrong e tag.", + ) + return false + } } val zappedNote = pending.zappedNote diff --git a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/services/nwc/NwcPaymentTracker.kt b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/services/nwc/NwcPaymentTracker.kt index a133912b3a..aa5319233b 100644 --- a/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/services/nwc/NwcPaymentTracker.kt +++ b/commons/src/jvmAndroid/kotlin/com/vitorpamplona/amethyst/commons/services/nwc/NwcPaymentTracker.kt @@ -33,18 +33,28 @@ import java.util.concurrent.ConcurrentHashMap * for the core request/response matching logic. * * Flow: - * 1. When sending payment request: [registerRequest] stores callback - * 2. When response arrives: [onResponseReceived] retrieves and removes pending request + * 1. When sending payment request: [registerRequest] stores callback and the + * expected wallet-service pubkey + * 2. When response arrives: [onResponseReceived] retrieves the pending request + * only if the response author matches the expected wallet-service pubkey * 3. Caller invokes callback and links notes via Note.addZapPayment() + * + * Author verification is required because the relay subscription filters by + * request id only (matching Primal's filter shape). An attacker who observes + * the request on the relay could otherwise forge a response with their own + * keypair and trick the client into displaying attacker-controlled data. */ class NwcPaymentTracker { /** * Data for a pending payment request. * + * @property expectedServicePubkey Pubkey of the wallet service we sent + * the request to. Only a response signed by this key is accepted. * @property zappedNote The note being zapped, if payment is for a zap * @property onResponse Callback to invoke when wallet responds */ data class PendingRequest( + val expectedServicePubkey: HexKey, val zappedNote: Note?, val onResponse: suspend (LnZapPaymentResponseEvent) -> Unit, ) @@ -55,27 +65,57 @@ class NwcPaymentTracker { * Registers a pending payment request. * * @param requestId Event ID of the LnZapPaymentRequestEvent + * @param expectedServicePubkey Wallet service pubkey from the request's `p` tag * @param zappedNote The note being zapped (null if not a zap payment) * @param onResponse Callback invoked when response arrives */ fun registerRequest( requestId: HexKey, + expectedServicePubkey: HexKey, zappedNote: Note?, onResponse: suspend (LnZapPaymentResponseEvent) -> Unit, ) { - awaitingRequests[requestId] = PendingRequest(zappedNote, onResponse) + awaitingRequests[requestId] = PendingRequest(expectedServicePubkey, zappedNote, onResponse) + } + + /** Outcome of matching a response event against the pending-requests table. */ + sealed interface MatchResult { + /** No pending request for this request id (or the id was null). */ + data object NoMatch : MatchResult + + /** A pending request exists but the response author does not match. */ + data class WrongAuthor( + val expected: HexKey, + val actual: HexKey, + ) : MatchResult + + /** Response is authentic; the pending request has been removed. */ + data class Matched( + val pending: PendingRequest, + ) : MatchResult } /** - * Called when a payment response event is received. - * Retrieves and removes the pending request for the given request ID. + * Looks up the pending request for the given response. Only consumes (and + * returns [MatchResult.Matched]) when the response author matches the + * stored [PendingRequest.expectedServicePubkey] — a forged response is + * left in the map so the legitimate one can still resolve it. * - * @param requestId The 'e' tag from the response, pointing to original request - * @return PendingRequest if found, null otherwise + * @param requestId The `e` tag from the response, pointing to original request + * @param responseAuthor The `pubkey` field of the response event */ - fun onResponseReceived(requestId: HexKey?): PendingRequest? { - if (requestId == null) return null - return awaitingRequests.remove(requestId) + fun onResponseReceived( + requestId: HexKey?, + responseAuthor: HexKey, + ): MatchResult { + if (requestId == null) return MatchResult.NoMatch + val pending = awaitingRequests[requestId] ?: return MatchResult.NoMatch + if (pending.expectedServicePubkey != responseAuthor) { + return MatchResult.WrongAuthor(pending.expectedServicePubkey, responseAuthor) + } + // Author matches — atomically remove and return. + val removed = awaitingRequests.remove(requestId) ?: return MatchResult.NoMatch + return MatchResult.Matched(removed) } /** diff --git a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCache.kt b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCache.kt index 6a67fc0865..2a52e4b5d5 100644 --- a/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCache.kt +++ b/desktopApp/src/jvmMain/kotlin/com/vitorpamplona/amethyst/desktop/cache/DesktopLocalCache.kt @@ -512,6 +512,11 @@ class DesktopLocalCache : ICacheProvider { wasVerified: Boolean = false, ): Boolean { if (!wasVerified && !justVerify(event)) return false + val expectedServicePubkey = + event.walletServicePubKey() ?: run { + Log.w("DesktopLocalCache") { "NWC request ${event.id} has no `p` tag; cannot register for response." } + return false + } val note = getOrCreateNote(event.id) val author = getOrCreateUser(event.pubKey) @@ -523,7 +528,7 @@ class DesktopLocalCache : ICacheProvider { relay?.let { note.addRelay(it) } zappedNote?.addZapPayment(note, null) - paymentTracker.registerRequest(event.id, zappedNote, onResponse) + paymentTracker.registerRequest(event.id, expectedServicePubkey, zappedNote, onResponse) return true } @@ -543,7 +548,18 @@ class DesktopLocalCache : ICacheProvider { ): Boolean { if (!wasVerified && !justVerify(event)) return false val requestId = event.requestId() - val pending = paymentTracker.onResponseReceived(requestId) ?: return false + val pending = + when (val match = paymentTracker.onResponseReceived(requestId, event.pubKey)) { + is NwcPaymentTracker.MatchResult.Matched -> match.pending + is NwcPaymentTracker.MatchResult.WrongAuthor -> { + Log.w("DesktopLocalCache") { + "Rejecting NWC response ${event.id}: expected author ${match.expected} but event was signed by ${match.actual}. " + + "This may be a spoofed reply — keeping the request pending for the legitimate wallet response." + } + return false + } + NwcPaymentTracker.MatchResult.NoMatch -> return false + } val requestNote = requestId?.let { getNoteIfExists(it) } val note = getOrCreateNote(event.id)