From fb9b70456616e157a39ddba14c38a316c5dfab8f Mon Sep 17 00:00:00 2001 From: Claude Date: Sun, 20 Sep 2026 21:30:55 +0000 Subject: [PATCH] fix(nwc): two lenient-reader bugs that answered with a wrong number MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Audit of the branch before merge. Both are in the readers behind the NIP-47 and CLINK parsers, where every byte comes from somebody else's wallet, and both break the one rule that file states: a field of the wrong shape reads as null and the rest of the message still parses — never a wrong value. * intOrNull narrowed with Long.toInt(), which truncates to the low 32 bits. `"limit":4294967296` came back 0 and `"offset":3000000000` came back negative. A plausible-looking wrong number is the one outcome worse than no number, and these feed list_transactions paging and CLINK error codes. Out of Int range now reads as null like every other unrepresentable value. * doubleOrNull accepted "NaN" and "Infinity", which Kotlin parses happily. A NaN read out of a peer's JSON propagates silently through every comparison it touches. Finite values only. Both were written as failing tests first and watched fail (LenientJsonTest, new), then fixed. The malformed-input suites for NIP-47 and CLINK still pass. Also records, in the JacksonMapper serializer guard, the two OptimizedSerializables deliberately not listed there — CLINK's SatRange, only ever written as a field by its parent message's kotlinx serializer, and NIP-55's IntentResult, which goes through JsonMapperNip55. Neither reaches that mapper today; both would throw if they did, and the next person should not have to re-derive why that is fine. Co-Authored-By: Claude Opus 5 Claude-Session: https://claude.ai/code/session_01DEoxktEZyTrAS33vVZBiwm --- .../kotlinSerialization/LenientJson.kt | 20 ++++- .../kotlinSerialization/LenientJsonTest.kt | 75 +++++++++++++++++++ .../quartz/nip01Core/jackson/JacksonMapper.kt | 6 ++ 3 files changed, 99 insertions(+), 2 deletions(-) create mode 100644 quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/kotlinSerialization/LenientJsonTest.kt diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/kotlinSerialization/LenientJson.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/kotlinSerialization/LenientJson.kt index 646452e96f..a205ca07b5 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/kotlinSerialization/LenientJson.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nip01Core/kotlinSerialization/LenientJson.kt @@ -89,9 +89,25 @@ fun JsonObject.longOrNull(key: String): Long? { return raw.toLongOrNull() ?: raw.toDoubleOrNull()?.takeIf { it.isFinite() }?.toLong() } -fun JsonObject.intOrNull(key: String): Int? = longOrNull(key)?.toInt() +/** + * Out of Int range reads as null, not as the low 32 bits. `Long.toInt()` truncates: + * a `limit` of 2^32 would come back 0 and an `offset` of 3_000_000_000 negative. + * Every other reader here answers a value it cannot represent with null, and a + * plausible-looking wrong number is the one outcome worse than no number. + */ +fun JsonObject.intOrNull(key: String): Int? = longOrNull(key)?.takeIf { it >= Int.MIN_VALUE.toLong() && it <= Int.MAX_VALUE.toLong() }?.toInt() -fun JsonObject.doubleOrNull(key: String): Double? = this[key].asPrimitiveOrNull()?.content?.toDoubleOrNull() +/** + * Finite values only. `"NaN"` and `"Infinity"` parse as Doubles in Kotlin, and a + * NaN read out of a peer's JSON propagates silently through every comparison it + * touches — it is not a number the caller can do anything with. + */ +fun JsonObject.doubleOrNull(key: String): Double? = + this[key] + .asPrimitiveOrNull() + ?.content + ?.toDoubleOrNull() + ?.takeIf { it.isFinite() } /** Accepts `true`/`false`, `"true"`/`"false"`, and the `1`/`0` some wallets send. */ fun JsonObject.booleanOrNull(key: String): Boolean? { diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/kotlinSerialization/LenientJsonTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/kotlinSerialization/LenientJsonTest.kt new file mode 100644 index 0000000000..61941cc498 --- /dev/null +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nip01Core/kotlinSerialization/LenientJsonTest.kt @@ -0,0 +1,75 @@ +/* + * Copyright (c) 2025 Vitor Pamplona + * + * Permission is hereby granted, free of charge, to any person obtaining a copy of + * this software and associated documentation files (the "Software"), to deal in + * the Software without restriction, including without limitation the rights to use, + * copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the + * Software, and to permit persons to whom the Software is furnished to do so, + * subject to the following conditions: + * + * The above copyright notice and this permission notice shall be included in all + * copies or substantial portions of the Software. + * + * THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR + * IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS + * FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR + * COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN + * AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION + * WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + */ +package com.vitorpamplona.quartz.nip01Core.kotlinSerialization + +import kotlinx.serialization.json.Json +import kotlinx.serialization.json.JsonObject +import kotlin.test.Test +import kotlin.test.assertEquals +import kotlin.test.assertNull + +/** + * The readers behind the NIP-47 and CLINK parsers, where every byte comes from + * somebody else's wallet. Their contract is that a field of the wrong shape reads + * as null and the rest of the message still parses — never a wrong value. + */ +class LenientJsonTest { + private fun obj(json: String) = Json.parseToJsonElement(json) as JsonObject + + @Test + fun intOutOfRangeReadsAsNullRatherThanWrapping() { + // 2^32 truncates to 0 and 3_000_000_000 to a negative int if the Long is + // narrowed with toInt(). A limit of 0 or a negative offset is worse than + // no value at all: it is a plausible-looking wrong answer. + assertNull(obj("""{"limit":4294967296}""").intOrNull("limit")) + assertNull(obj("""{"offset":3000000000}""").intOrNull("offset")) + assertNull(obj("""{"n":-3000000000}""").intOrNull("n")) + } + + @Test + fun intInRangeStillReads() { + assertEquals(42, obj("""{"n":42}""").intOrNull("n")) + assertEquals(Int.MAX_VALUE, obj("""{"n":2147483647}""").intOrNull("n")) + assertEquals(Int.MIN_VALUE, obj("""{"n":-2147483648}""").intOrNull("n")) + // the same string/float tolerance longOrNull has + assertEquals(12, obj("""{"n":"12"}""").intOrNull("n")) + assertEquals(12, obj("""{"n":12.0}""").intOrNull("n")) + } + + @Test + fun longAcceptsTheThreeShapesWalletsSend() { + assertEquals(12L, obj("""{"n":12}""").longOrNull("n")) + assertEquals(12L, obj("""{"n":"12"}""").longOrNull("n")) + assertEquals(12L, obj("""{"n":12.0}""").longOrNull("n")) + assertNull(obj("""{"n":{"nested":1}}""").longOrNull("n")) + assertNull(obj("""{"n":null}""").longOrNull("n")) + assertNull(obj("""{}""").longOrNull("n")) + } + + @Test + fun doubleRejectsNonFiniteText() { + // "NaN" and "Infinity" parse as Doubles in Kotlin but are not values any + // caller can do arithmetic with. + assertNull(obj("""{"n":"NaN"}""").doubleOrNull("n")) + assertNull(obj("""{"n":"Infinity"}""").doubleOrNull("n")) + assertEquals(1.5, obj("""{"n":1.5}""").doubleOrNull("n")) + } +} diff --git a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt index 441d1005a4..c02d6577c3 100644 --- a/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt +++ b/quartz/src/jvmAndroid/kotlin/com/vitorpamplona/quartz/nip01Core/jackson/JacksonMapper.kt @@ -199,6 +199,12 @@ class JacksonMapper { * [BunkerRequest] and [BunkerResponse] are named instead of their [BunkerMessage] * parent on purpose: the parent has no serializer of its own, so a third subclass * should fail here rather than quietly bean-serialize. + * + * Two OptimizedSerializables are deliberately absent, and both would throw if + * they ever arrived here: CLINK's SatRange, which is only ever written as a + * field of an Offer/Debit/Manage message by that message's kotlinx serializer, + * and NIP-55's IntentResult, which goes through JsonMapperNip55 and its own + * registered serializer. Neither reaches this mapper today. */ private fun checkSerializable(value: OptimizedSerializable) { val hasSerializer =