invoice with a huge expiry crashes or busy-loops lightningd; openchannel_bump on a V1 channel asserts before its own typed errors - #9478
Open
Amperstrand wants to merge 2 commits into
Conversation
added 2 commits
September 5, 2026 14:26
invoice's `expiry` parameter is an unclamped param_u64, and far-future values break the daemon in two different ways: - expiry >= 2^60 needs more bits than push_varlen_field() can encode in the bolt11 `x` field, so bolt11_encode() aborts the whole daemon (FATAL SIGNAL 6). - far below that (anywhere past ~584k years), the invoice expiration timer's nanosecond-grain u64 counter overflows: install_expiration_timer() arms a timer that reads as already due, trigger_expiration() finds nothing expired, re-arms, and the daemon busy-loops at 100% CPU with the RPC reply left racing the storm. Refuse at the parameter stage instead: expiry >= 2^32 seconds (~136 years) returns JSONRPC2_INVALID_PARAMS, keeping a wide margin under both limits. 2^32 - 1 still works. Changelog-Fixed: lightningd: fix crash (`FATAL SIGNAL 6`) and a 100% CPU busy-loop when calling `invoice` with an `expiry` too far in the future (now refused above 2^32 seconds).
json_openchannel_bump() computed the BOLT-2 25/24 feerate ramp and
asserted next_feerate_min > last_feerate_perkw BEFORE the
channel-state gates. channel_last_funding_feerate() returns 0 for a
channel without an in-flight funding transaction - every V1 channel -
so the assert evaluated `0 > 0` and aborted the daemon one screen
above the typed errors written for exactly this case ("Channel not
eligible to init RBF", "No inflight for this channel exists").
Move the ramp computation below the state gates: the assert is then
only reached when an inflight exists and last_feerate_perkw is nonzero
by construction, and the existing typed errors become reachable again.
Changelog-Fixed: lightningd: fix crash (`FATAL SIGNAL 6`) when calling `openchannel_bump` on a channel without an in-flight RBF attempt (e.g. any V1 channel).
Author
|
Verification update against the v26.06.7 signed release binaries (fresh regtest rig,
Keeping both commits in place for now; say the word and I'll trim to just the invoice fix. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two RPC-reachable daemon failures at current master, both triggerable by a single call from any authenticated RPC client (no peer, no funds movement, no unusual config). Two self-contained commits, each carrying its own regression test.
1.
invoicewith a far-futureexpiry— abort or 100% CPU busy-loopinvoice'sexpiryparameter is an unclampedparam_u64, and two ranges of far-future values break the daemon in different ways:expiry >= 2^60needs more bits thanpush_varlen_field()(common/bolt11.c) can encode in thexfield, sobolt11_encode()aborts the whole daemon (FATAL SIGNAL 6).2^60 - 1, exactly 60 bits, encodes fine.Far below that, the invoice expiration timer breaks:
install_expiration_timer()(wallet/invoices.c) arms a timer forMIN(expiry_time) - now, but the timer's nanosecond-grain u64 counter (time_to_grains():tv_sec * 1e6,TIMER_GRANULARITY1000) overflows for relative delays beyond ~1.845e13 seconds (~584k years). The wrapped timer reads as already due:trigger_expiration()finds nothing expired, re-arms, and the daemon busy-loops at ~100% CPU inside the timers/sqlite churn, with the RPC reply racing the storm (a wedged-but-alive daemon — visible in a backtrace astimer_expired -> trigger_expiration -> expired_ids -> sqlite3).Fix: refuse at the parameter stage —
expiry >= 2^32seconds (~136 years) returnsJSONRPC2_INVALID_PARAMS("expiry must be below 2^32 seconds (~136 years)"), keeping a wide margin under both limits.2^32 - 1still works and is pinned by the test.2.
openchannel_bumpon a channel without an RBF inflight —assert(0 > 0)json_openchannel_bump()computes the BOLT-2 25/24 feerate ramp and assertsnext_feerate_min > last_feerate_perkwBEFORE the channel-state gates.channel_last_funding_feerate()returns 0 for a channel without an in-flight funding transaction — every V1fundchannelchannel — so the assert evaluates0 > 0and kills the daemon one screen above the honest typed errors ("Channel not eligible to init RBF" / "No inflight for this channel exists") that were written for exactly this call.Fix: move the ramp computation below the state gates. The assert is then only reached when an inflight exists (so
last_feerate_perkwis nonzero by construction) and the existing typed errors become reachable again.Alternatives considered (for the expiry side)
install_expiration_timer()could skip arming when the relative delay exceeds the representable timer range. That would also protect any future caller that feeds a huge delay into the same helper, but it silently leaves invoices unexpired rather than telling the caller their input is absurd, and it touches the timer contract for a single known caller. Happy to add it as belt-and-braces on top if maintainers prefer both.Tests
tests/test_invoices.py::test_invoice_expiry_too_large: exact boundary2^32 - 1must still work;2^32must return-32602with the daemon alive (getinfo()asserted on every path).tests/test_opening.py::test_openchannel_bump_no_inflight: well-formed request against a funded V1 channel must return312 FUNDING_STATE_INVALID(the pre-existing typed error), daemon alive.On vanilla master both corners die with
FATAL SIGNAL 6; with these commits everything passes, including the adjacent suites:test_invoice_expiry, and the real dual-funded RBF flowstest_v2_rbf_single/test_v2_rbf_abort_retry(which exercise the reordered ramp on the legitimate path, run underEXPERIMENTAL_DUAL_FUND=1).Both sites verified present in current master and in
v26.06.6.Cross-implementation note
lnd, eclair and electrum all return typed errors on caller-induced funding/invoice corners; neither peer implementation aborts the process on RPC input.
Checklist
Changelog-Fixed:trailers).tools/lightning-downgrade(no persistent/state changes — refusal and reorder only).