mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-08-08 23:54:39 +00:00
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 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpgpWLKzgD7vS9Fs4CXTR3
This commit is contained in:
+3
-3
@@ -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<String, Any?>? {
|
||||
val offer = Bolt12Offer.parse(raw) ?: return null
|
||||
|
||||
+6
-3
@@ -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
|
||||
|
||||
+3
@@ -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)
|
||||
|
||||
@@ -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
|
||||
|
||||
+12
@@ -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 =
|
||||
|
||||
+5
@@ -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'")
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
Reference in New Issue
Block a user