lightningd: fix crash on invoice_payment_hook_done
What changed, and why it matters
This commit fixes a crash in the Core Lightning node software. The crash happens when an invoice is deleted while a plugin's 'invoice_payment' hook is still processing a payment that came in through an on-chain fallback address. By the time the hook finishes, the internal data structure it expected to use (the HTLC set) had already been cleared, causing the node to read invalid memory and crash. The fix simply checks whether that data still exists before using it.
Apply the patch. The fix is small and targeted; it only adds a NULL guard before calling htlc_set_fail(). Operators running nodes with plugins that use the invoice_payment hook and have invoices-onchain-fallback enabled should upgrade. No immediate workaround is required unless the feature combination is actively used.
Security signals we found
Invalid memory read (use-after-free/NULL-deref) in htlc_set_fail_ triggered from invoice_payment_hooks_done
Race condition between delinvoice and asynchronous invoice_payment hook completion
On-chain fallback settlement path involved
Crash of lightningd daemon (denial of service)
Regression test added to prevent reintroduction
Evidence from the diff
The patch addresses a use-after-free / NULL-deref style crash in invoice_payment_hooks_done(). When an on-chain fallback payment triggers the invoice_payment plugin hook and the invoice is deleted via delinvoice before the hook returns, payload->set can be NULL by the time invoice_payment_hooks_done() runs. The original code unconditionally called htlc_set_fail(payload->set, NULL), leading to an invalid read inside htlc_set_fail_() (Valgrind reports Address 0x38). The fix guards the call with if (payload->set). A regression test reproduces the race by holding the hook, deleting the invoice, then releasing the hook.
Changed components
lightningd/invoice.clightningd/htlc_set.cplugin hook subsystem (invoice_payment hook)on-chain invoice fallback settlement flowInspect captured patch +48 / −1
### lightningd/invoice.c
@@ -273,7 +273,8 @@ invoice_payment_hooks_done(struct invoice_payment_hook_payload *payload STEALS)
/* If invoice gets paid meanwhile (plugin responds out-of-order?) then
* we can also fail */
if (!invoices_find_by_label(ld->wallet->invoices, &inv_dbid, payload->label)) {
- htlc_set_fail(payload->set, NULL);
+ if (payload->set)
+ htlc_set_fail(payload->set, NULL);
return;
}
### tests/test_invoices.py
@@ -888,6 +888,52 @@ def test_unified_invoices(node_factory, bitcoind):
assert(txid == res['paid_outpoint']['txid'])
+def test_onchain_invoice_delinvoice_during_payment_hook(node_factory, bitcoind):
+ """delinvoice while onchain invoice_payment hook is pending must not crash."""
+ # Absolute path: inline plugins run in the test process (not lightning-dir).
+ unhold = [None]
+
+ def setup(plugin):
+ @plugin.hook("invoice_payment")
+ def on_payment(payment, plugin, **kwargs):
+ plugin.log("holding invoice_payment for label={}".format(payment["label"]))
+ while not os.path.exists(unhold[0]):
+ time.sleep(0.1)
+ plugin.log(
+ "releasing invoice_payment for label={}".format(payment["label"])
+ )
+ return {"result": "continue"}
+
+ l1 = node_factory.get_node(
+ options={"invoices-onchain-fallback": None}, inline_plugin=setup
+ )
+ unhold[0] = os.path.join(l1.daemon.lightning_dir, TEST_NETWORK, "unhold")
+ amount_sat = 1000
+ inv = l1.rpc.invoice(
+ amount_sat * 1000, "inv1", "test_onchain_invoice_delinvoice_during_payment_hook"
+ )
+ b11 = l1.rpc.decode(inv["bolt11"])
+ assert len(b11["fallbacks"]) == 1
+ addr = b11["fallbacks"][0]["addr"]
+
+ # Pay the on-chain fallback while the hook holds resolution.
+ bitcoind.rpc.sendtoaddress(addr, amount_sat / 10**8)
+ bitcoind.generate_block(1)
+
+ l1.daemon.wait_for_log(r"holding invoice_payment for label=inv1")
+ assert only_one(l1.rpc.listinvoices("inv1")["invoices"])["status"] == "unpaid"
+
+ # Delete the unpaid invoice while the hook is still pending.
+ l1.rpc.delinvoice("inv1", "unpaid")
+
+ # Let the hook finish; lightningd must survive the stale reply.
+ open(unhold[0], "w").close()
+ l1.daemon.wait_for_log(r"releasing invoice_payment for label=inv1")
+
+ # RPC still works => no restartable crash from invoice_payment_hooks_done.
+ assert l1.rpc.listinvoices("inv1") == {"invoices": []}
+
+
def test_expiry_startup_crash(node_factory, bitcoind):
"""We crash trying to expire invoice on startup"""
l1 = node_factory.get_node()Why this scored 66/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.