From 9caf330879de70da71d0c448347b82db4dda2d29 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 25 Jul 2026 03:00:51 +0000 Subject: [PATCH] fix(bolt12): don't throw on an oversized tu64; cover proof_note path Audit of the compressed-proof work found one real defect and one coverage gap. Defect: a hostile BOLT12 proof/offer can carry a 9+ byte `invoice_amount` (or any tu64 field) that parses as a valid TLV. `TlvStream.tu64` then called the strict `Bolt12Values.tu64`, which throws `require(size <= 8)`. On the `amy bolt12 verify` path (`Bolt12ZapActions.validate`, no surrounding catch) that surfaced as an uncaught exception and abnormal exit instead of a clean `Invalid`; the Android ingest path was already contained by LocalCache's broad catch. Make the nullable stream accessor `TlvStream.tu64` return null for an over-8-byte value so every amount read (invoice_amount, invreq_amount, offer amount) degrades to a clean rejection. Regression-tested at the codec level. Coverage: the writer's `proof_note` (1005) branch and the `with_note` vector's note were never exercised. Add a `Bolt12PayerProof.proofNote()` reader and thread the vector's note through the writer round-trip so 1005 is asserted. The forged-proof, DoS, and reconstruction-accounting paths were reviewed and found sound (the reconstructed root is only ever a BIP-340 message; the NIP offer-binding gate still pins invoice_node_id to the offer's issuer). Co-Authored-By: Claude Opus 4.8 Claude-Session: https://claude.ai/code/session_01SpgpWLKzgD7vS9Fs4CXTR3 --- .../amethyst/commons/actions/Bolt12ZapActions.kt | 6 +++--- .../amethyst/commons/actions/Bolt12ZapActionsTest.kt | 9 ++++++--- .../nipXXBolt12Zaps/bolt12/Bolt12PayerProof.kt | 3 +++ .../quartz/nipXXBolt12Zaps/bolt12/Tlv.kt | 9 +++++++-- .../quartz/nipXXBolt12Zaps/bolt12/TlvTest.kt | 12 ++++++++++++ .../verify/Bolt12PayerProofVectorTest.kt | 5 +++++ 6 files changed, 36 insertions(+), 8 deletions(-) diff --git a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/Bolt12ZapActions.kt b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/Bolt12ZapActions.kt index bcf6cdce3c..3875931c94 100644 --- a/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/Bolt12ZapActions.kt +++ b/commons/src/commonMain/kotlin/com/vitorpamplona/amethyst/commons/actions/Bolt12ZapActions.kt @@ -55,9 +55,9 @@ object Bolt12ZapActions { /** * Decode a BOLT12 offer (`lno1…`) to its interesting fields, or null when unparseable. - * A field read can still throw on a well-encoded-but-malformed TLV (e.g. an amount - * value longer than 8 bytes), so the whole field extraction is guarded to honor the - * null contract rather than leak an exception to the caller. + * Individual field reads degrade to null on malformed values (e.g. an amount longer + * than 8 bytes), so a structurally-valid offer still decodes without its bad field; + * the extraction stays guarded as defense-in-depth against any future throwing read. */ fun decodeOffer(raw: String): Map? { val offer = Bolt12Offer.parse(raw) ?: return null diff --git a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/Bolt12ZapActionsTest.kt b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/Bolt12ZapActionsTest.kt index fdccafba05..fabbf01868 100644 --- a/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/Bolt12ZapActionsTest.kt +++ b/commons/src/commonTest/kotlin/com/vitorpamplona/amethyst/commons/actions/Bolt12ZapActionsTest.kt @@ -71,15 +71,18 @@ class Bolt12ZapActionsTest { } @Test - fun decodeOfferReturnsNullForAParseableButMalformedAmount() { + fun decodeOfferOmitsAMalformedAmountWithoutThrowing() { // The TLV stream parses (ascending type, valid length), but the amount value is - // 9 bytes — reading it throws in tu64. decodeOffer must honor its null contract. + // 9 bytes — too long for a tu64. Reading it must not throw: the offer still + // decodes, just without an `amount_msat`. val bad = Bolt12Bech32.encode( Bolt12Bech32.OFFER_HRP, TlvStream(listOf(TlvRecord(Bolt12Offer.TYPE_AMOUNT, ByteArray(9) { 1 }))).encode(), ) - assertNull(Bolt12ZapActions.decodeOffer(bad)) + val fields = Bolt12ZapActions.decodeOffer(bad) + assertTrue(fields != null) + assertNull(fields["amount_msat"]) } @Test diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/Bolt12PayerProof.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/Bolt12PayerProof.kt index bbcbadca72..3cbf2f0fa2 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/Bolt12PayerProof.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/Bolt12PayerProof.kt @@ -56,6 +56,9 @@ class Bolt12PayerProof( fun proofPreimage(): ByteArray? = tlv.value(TYPE_PROOF_PREIMAGE) + /** The optional free-text `proof_note` (1005) a challenge-response verifier may request. */ + fun proofNote(): String? = tlv.value(TYPE_PROOF_NOTE)?.decodeToString() + fun proofOmittedTlvs(): ByteArray? = tlv.value(TYPE_PROOF_OMITTED_TLVS) fun proofMissingHashes(): ByteArray? = tlv.value(TYPE_PROOF_MISSING_HASHES) diff --git a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/Tlv.kt b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/Tlv.kt index 2a7285d679..7d166e4980 100644 --- a/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/Tlv.kt +++ b/quartz/src/commonMain/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/Tlv.kt @@ -147,8 +147,13 @@ class TlvStream( fun has(type: Long): Boolean = get(type) != null - /** The truncated-uint64 value of a record, or null if absent. */ - fun tu64(type: Long): Long? = value(type)?.let { Bolt12Values.tu64(it) } + /** + * The truncated-uint64 value of a record, or null if absent **or malformed**. + * A `tu64` is at most 8 bytes; a longer value is invalid encoding, so this + * returns null rather than throwing — untrusted proofs/offers reach this on the + * validation path and must degrade to a clean rejection, not an exception. + */ + fun tu64(type: Long): Long? = value(type)?.let { if (it.size > 8) null else Bolt12Values.tu64(it) } fun encode(): ByteArray { var size = 0 diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/TlvTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/TlvTest.kt index c7d67ce438..5308744b5c 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/TlvTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/bolt12/TlvTest.kt @@ -64,6 +64,18 @@ class TlvTest { } } + @Test + fun tu64FieldReturnsNullForAnOversizedValueInsteadOfThrowing() { + // A tu64 value must be at most 8 bytes. A hostile proof/offer can carry a 9-byte + // amount that parses as a TLV; reading it via the stream accessor must degrade to + // null (a clean rejection on the validation path), not throw. + val stream = TlvStream(listOf(TlvRecord(170, ByteArray(9) { 1 }))) + val decoded = TlvStream.read(stream.encode()) + assertNull(decoded.tu64(170)) + // The exact-8-byte boundary still decodes. + assertEquals(Long.MAX_VALUE, TlvStream(listOf(TlvRecord(170, Bolt12Values.tu64ToBytes(Long.MAX_VALUE)))).tu64(170)) + } + @Test fun tlvStreamRoundTrips() { val records = diff --git a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/verify/Bolt12PayerProofVectorTest.kt b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/verify/Bolt12PayerProofVectorTest.kt index 396ca38ef6..449d4cc12e 100644 --- a/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/verify/Bolt12PayerProofVectorTest.kt +++ b/quartz/src/commonTest/kotlin/com/vitorpamplona/quartz/nipXXBolt12Zaps/verify/Bolt12PayerProofVectorTest.kt @@ -108,10 +108,12 @@ class Bolt12PayerProofVectorTest { // Deterministic compression fields don't depend on the signatures, so // dummy signers suffice; we read the minted proof back and compare. + val note = obj["input"]!!.jsonObject["note"]?.jsonPrimitive?.content val minted = Bolt12ProofBuilder.build( invoiceFields = invoiceFields, preimage = Hex.decode(obj["input"]!!.jsonObject["preimage"]!!.jsonPrimitive.content), + proofNote = note, signInvoiceDigest = { ByteArray(64) }, signProofDigest = { ByteArray(64) }, ) @@ -125,6 +127,9 @@ class Bolt12PayerProofVectorTest { val expectedLeaves = working["proof_leaf_hashes"]!!.jsonArray.map { it.jsonPrimitive.content } assertEquals(expectedLeaves, proof.leafHashList()!!.map { Hex.encode(it) }, "proof_leaf_hashes mismatch for '$name'") + + // The optional proof_note (1005) must round-trip when the vector carries one. + assertEquals(note, proof.proofNote(), "proof_note mismatch for '$name'") } } }