mirror of
https://github.com/vitorpamplona/amethyst.git
synced 2026-10-06 03:38:23 +00:00
fix(bolt12): audit fixes — verify crash, decode hardening, send robustness
- amy bolt12 verify: the id-only query with a <Bolt12ZapEvent> type crashed with ClassCastException when the id pointed at a non-9736 event. Constrain the filter to kind 9736 and cast defensively (query<Event>() as?), returning a clean not_found instead. - Bolt12ZapActions.decodeOffer/decodeProof: a parseable bech32 with an over-8-byte amount TLV threw in tu64 on field read instead of honoring the null contract; guarded the field extraction in runCatching. Adds a malformed-amount regression test. - Account.sendBolt12Zap: wrap the NWC response callback in try/catch/finally so a post-payment receipt-assembly failure (e.g. a remote signer error) steps progress and surfaces "paid, no receipt" instead of vanishing as an uncaught coroutine exception. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01SpgpWLKzgD7vS9Fs4CXTR3
This commit is contained in:
@@ -1505,27 +1505,39 @@ class Account(
|
||||
|
||||
sendNwcRequest(PayMethod.create("bitcoin:?lno=$offer", amountMillisats, payerNote)) { response ->
|
||||
scope.launch {
|
||||
when (response) {
|
||||
is PaySuccessResponse -> {
|
||||
val proof = response.result?.payer_proof
|
||||
if (proof.isNullOrBlank()) {
|
||||
onError(R.string.bolt12_zap_paid_no_receipt, null)
|
||||
} else {
|
||||
val zap = Bolt12ZapBuilder.buildZap(zapSigner, intent, proof, anonymous)
|
||||
if (cache.bolt12ZapValidator.validate(zap, verifyEventSignature = false) is Bolt12ZapValidation.Valid) {
|
||||
cache.justConsumeMyOwnEvent(zap)
|
||||
client.publish(zap, computeRelayListToBroadcast(zap))
|
||||
// try/finally so a failure while assembling/publishing the receipt (e.g. a
|
||||
// remote signer error) still steps progress and surfaces an error, instead
|
||||
// of vanishing as an uncaught coroutine exception. The payment already
|
||||
// settled at this point, so such a failure means "paid, no receipt".
|
||||
try {
|
||||
when (response) {
|
||||
is PaySuccessResponse -> {
|
||||
val proof = response.result?.payer_proof
|
||||
if (proof.isNullOrBlank()) {
|
||||
onError(R.string.bolt12_zap_paid_no_receipt, null)
|
||||
} else {
|
||||
onError(R.string.bolt12_zap_invalid_receipt, null)
|
||||
val zap = Bolt12ZapBuilder.buildZap(zapSigner, intent, proof, anonymous)
|
||||
if (cache.bolt12ZapValidator.validate(zap, verifyEventSignature = false) is Bolt12ZapValidation.Valid) {
|
||||
cache.justConsumeMyOwnEvent(zap)
|
||||
client.publish(zap, computeRelayListToBroadcast(zap))
|
||||
} else {
|
||||
onError(R.string.bolt12_zap_invalid_receipt, null)
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
is IErrorResponseLike -> onError(R.string.bolt12_payment_failed, response.errorMessage())
|
||||
|
||||
else -> onError(R.string.bolt12_zap_paid_no_receipt, null)
|
||||
}
|
||||
|
||||
is IErrorResponseLike -> onError(R.string.bolt12_payment_failed, response.errorMessage())
|
||||
|
||||
else -> onError(R.string.bolt12_zap_paid_no_receipt, null)
|
||||
} catch (e: CancellationException) {
|
||||
throw e
|
||||
} catch (e: Exception) {
|
||||
Log.w("Account", "BOLT12 zap receipt assembly failed after payment", e)
|
||||
onError(R.string.bolt12_zap_paid_no_receipt, null)
|
||||
} finally {
|
||||
onProcessed()
|
||||
}
|
||||
onProcessed()
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
@@ -25,6 +25,7 @@ import com.vitorpamplona.amethyst.cli.Context
|
||||
import com.vitorpamplona.amethyst.cli.DataDir
|
||||
import com.vitorpamplona.amethyst.cli.Output
|
||||
import com.vitorpamplona.amethyst.commons.actions.Bolt12ZapActions
|
||||
import com.vitorpamplona.quartz.nip01Core.core.Event
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.filters.Filter
|
||||
import com.vitorpamplona.quartz.nip01Core.relay.normalizer.NormalizedRelayUrl
|
||||
import com.vitorpamplona.quartz.nipXXBolt12Zaps.bolt12.Bolt12Bech32
|
||||
@@ -113,8 +114,13 @@ object Bolt12Commands {
|
||||
if (eventId.length != 64) return Output.error("bad_args", "event-id must be 64-hex")
|
||||
Context.open(dataDir).use { ctx ->
|
||||
ctx.prepare()
|
||||
// Constrain by kind AND cast defensively: the store filters by id alone and
|
||||
// returns whatever kind that id actually is, so querying <Bolt12ZapEvent>
|
||||
// directly would ClassCastException if the id points at a non-9736 event.
|
||||
val zap =
|
||||
ctx.store.query<Bolt12ZapEvent>(Filter(ids = listOf(eventId), limit = 1)).firstOrNull()
|
||||
ctx.store
|
||||
.query<Event>(Filter(kinds = listOf(Bolt12ZapEvent.KIND), ids = listOf(eventId), limit = 1))
|
||||
.firstOrNull() as? Bolt12ZapEvent
|
||||
?: return Output.error("not_found", "no kind:9736 event $eventId in local store; sync or fetch first")
|
||||
|
||||
when (val result = Bolt12ZapActions.validate(zap)) {
|
||||
|
||||
+32
-20
@@ -53,32 +53,44 @@ object Bolt12ZapActions {
|
||||
return if (Bolt12Bech32.isOffer(canonical)) canonical else null
|
||||
}
|
||||
|
||||
/** Decode a BOLT12 offer (`lno1…`) to its interesting fields, or null when unparseable. */
|
||||
/**
|
||||
* 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.
|
||||
*/
|
||||
fun decodeOffer(raw: String): Map<String, Any?>? {
|
||||
val offer = Bolt12Offer.parse(raw) ?: return null
|
||||
return buildMap {
|
||||
put("canonical", Bolt12Bech32.canonicalize(raw))
|
||||
offer.amount()?.let { put("amount_msat", it) }
|
||||
offer.currency()?.let { put("currency", it) }
|
||||
offer.description()?.let { put("description", it) }
|
||||
offer.issuerId()?.let { put("issuer_id", Hex.encode(it)) }
|
||||
put("has_paths", offer.hasPaths())
|
||||
}
|
||||
return runCatching {
|
||||
buildMap {
|
||||
put("canonical", Bolt12Bech32.canonicalize(raw))
|
||||
offer.amount()?.let { put("amount_msat", it) }
|
||||
offer.currency()?.let { put("currency", it) }
|
||||
offer.description()?.let { put("description", it) }
|
||||
offer.issuerId()?.let { put("issuer_id", Hex.encode(it)) }
|
||||
put("has_paths", offer.hasPaths())
|
||||
}
|
||||
}.getOrNull()
|
||||
}
|
||||
|
||||
/** Decode a BOLT12 payer proof (`lnp1…`) to its interesting fields, or null when unparseable. */
|
||||
/**
|
||||
* Decode a BOLT12 payer proof (`lnp1…`) to its interesting fields, or null when
|
||||
* unparseable. Guarded like [decodeOffer] against a malformed-but-encoded field.
|
||||
*/
|
||||
fun decodeProof(raw: String): Map<String, Any?>? {
|
||||
val proof = Bolt12PayerProof.parse(raw) ?: return null
|
||||
return buildMap {
|
||||
put("has_all_required_fields", proof.hasAllRequiredFields())
|
||||
put("compressed", proof.isCompressed())
|
||||
proof.invreqPayerNote()?.let { put("invreq_payer_note", it) }
|
||||
proof.invreqPayerId()?.let { put("invreq_payer_id", Hex.encode(it)) }
|
||||
proof.invoiceAmount()?.let { put("invoice_amount_msat", it) }
|
||||
proof.invoicePaymentHash()?.let { put("invoice_payment_hash", Hex.encode(it)) }
|
||||
proof.invoiceNodeId()?.let { put("invoice_node_id", Hex.encode(it)) }
|
||||
proof.offerIssuerId()?.let { put("offer_issuer_id", Hex.encode(it)) }
|
||||
}
|
||||
return runCatching {
|
||||
buildMap {
|
||||
put("has_all_required_fields", proof.hasAllRequiredFields())
|
||||
put("compressed", proof.isCompressed())
|
||||
proof.invreqPayerNote()?.let { put("invreq_payer_note", it) }
|
||||
proof.invreqPayerId()?.let { put("invreq_payer_id", Hex.encode(it)) }
|
||||
proof.invoiceAmount()?.let { put("invoice_amount_msat", it) }
|
||||
proof.invoicePaymentHash()?.let { put("invoice_payment_hash", Hex.encode(it)) }
|
||||
proof.invoiceNodeId()?.let { put("invoice_node_id", Hex.encode(it)) }
|
||||
proof.offerIssuerId()?.let { put("offer_issuer_id", Hex.encode(it)) }
|
||||
}
|
||||
}.getOrNull()
|
||||
}
|
||||
|
||||
/** The value a payer MUST put in the BOLT12 `invreq_payer_note` to bind a payment to [intent]. */
|
||||
|
||||
+12
@@ -70,6 +70,18 @@ class Bolt12ZapActionsTest {
|
||||
assertNull(Bolt12ZapActions.decodeOffer("lnp1garbage"))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun decodeOfferReturnsNullForAParseableButMalformedAmount() {
|
||||
// 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.
|
||||
val bad =
|
||||
Bolt12Bech32.encode(
|
||||
Bolt12Bech32.OFFER_HRP,
|
||||
TlvStream(listOf(TlvRecord(Bolt12Offer.TYPE_AMOUNT, ByteArray(9) { 1 }))).encode(),
|
||||
)
|
||||
assertNull(Bolt12ZapActions.decodeOffer(bad))
|
||||
}
|
||||
|
||||
@Test
|
||||
fun offerListRoundTrips() =
|
||||
runTest {
|
||||
|
||||
Reference in New Issue
Block a user