channeld: bound HTLC additions by our own splice-out amount
What the developer wrote
Authored by jaonoctus
channeld: bound HTLC additions by our own splice-out amount
While a splice is inflight, every HTLC has to fit on the commitment for
each funding candidate, so get_room_above_reserve() charges each side's
lowest splice change (lowest_splice_amnt) against its balance. But
update_view_from_inflights() set our side's bound from the inflight's
new funding total, which is positive, so it stayed at zero and our
splice-out was never charged.
We then admitted HTLCs that fit only the pre-splice commitment. Building
the post-splice commitment took our balance negative, channel_txs()
returned NULL, and channeld dereferenced it and crashed, then did the same
on every reconnect:
during the splice, payments were forwarded out through the channel
cumulatively they exceeded our post-splice balance (invalid on the
post-splice commitment)
...
channeld crashed building the post-splice commitment: our balance
went negative, the commitment build returned nothing, and channeld
dereferenced it
every reconnect rebuilds the same commitment, so it loops
Use the inflight's signed local change for our bound, as the remote side
already does. Also fix the REMOTE-view assignments, which compared
against one side's bound and wrote the other's.
Reported-by: Ken Sedgwick <ken@bonsai.com>
Changelog-Fixed: channeld: no longer crash-loops when an HTLC that only the pre-splice commitment can carry is added during a splice-out.
Co-Authored-By: Ken Sedgwick <ken@bonsai.com>
(cherry picked from commit 733c42e823bb204e6d91cca4f0450158842404e3)
What changed, and why it matters
This fix repairs a bug in Core Lightning's channel daemon that could crash and permanently loop-crash a node during a specific kind of Bitcoin channel upgrade called a 'splice-out.' While the splice was in progress, the program failed to subtract the node's outgoing splice amount when checking whether new forwarded payments would fit in the updated channel. It therefore allowed payments that were valid only for the old channel state. When it later tried to build the post-splice commitment transaction, the node's balance went negative, the builder returned nothing, and the daemon dereferenced that null pointer and crashed. On every reconnect it rebuilt the same bad commitment, so the node would crash again in a loop. The patch corrects the accounting so the splice-out amount is properly charged against the balance, and also fixes a related indexing mistake that mixed up local and remote views.
Apply the patch and backport it to maintained release branches. Nodes that perform splice-outs should upgrade before initiating splice-out operations. Monitor for crash-loop behavior during splice operations and consider disabling splice-out functionality until patched.
Security signals we found
Null-pointer dereference crash in channeld
Crash loop on reconnect due to deterministic bad commitment rebuild
Incorrect balance accounting during splice inflight
Cross-view index mismatch in lowest_splice_amnt assignment
Denial-of-service against a forwarding node during a splice-out
Evidence from the diff
In update_view_from_inflights(), the code previously used inflights[i]->amnt.satoshis as the local splice amount. That value is the new total funding, not the signed local change, so for a splice-out it remained positive and the local side’s lowest_splice_amnt[LOCAL] was never reduced. get_room_above_reserve() therefore did not charge the splice-out against the local balance, allowing HTLCs that fit only the pre-splice commitment. When channel_txs() later built the post-splice commitment, the local balance became negative, channel_txs() returned NULL, and channeld dereferenced it, crashing. The patch uses inflights[i]->splice_amnt (the signed local change) and fixes the REMOTE-view assignments so the correct side’s bound is compared and written.
Changed components
channeld/channeld.cupdate_view_from_inflights()splice_state->inflights handlingHTLC admission during spliceInspect captured patch +7 / −4
### channeld/channeld.c
@@ -3827,20 +3827,23 @@ static void update_view_from_inflights(struct peer *peer)
struct inflight **inflights = peer->splice_state->inflights;
for (size_t i = 0; i < tal_count(inflights); i++) {
- s64 splice_amnt = inflights[i]->amnt.satoshis; /* Raw: splicing */
+ /* Each side's balance change on this inflight's commitment,
+ * relative to the current funding: the same amounts
+ * channel_txs() applies when it builds that commitment. */
+ s64 splice_amnt = inflights[i]->splice_amnt;
s64 funding_diff = sats_diff(inflights[i]->amnt, peer->channel->funding_sats);
- s64 remote_splice_amnt = funding_diff - inflights[i]->splice_amnt;
+ s64 remote_splice_amnt = funding_diff - splice_amnt;
if (splice_amnt < peer->channel->view[LOCAL].lowest_splice_amnt[LOCAL])
peer->channel->view[LOCAL].lowest_splice_amnt[LOCAL] = splice_amnt;
- if (splice_amnt < peer->channel->view[REMOTE].lowest_splice_amnt[REMOTE])
+ if (splice_amnt < peer->channel->view[REMOTE].lowest_splice_amnt[LOCAL])
peer->channel->view[REMOTE].lowest_splice_amnt[LOCAL] = splice_amnt;
if (remote_splice_amnt < peer->channel->view[LOCAL].lowest_splice_amnt[REMOTE])
peer->channel->view[LOCAL].lowest_splice_amnt[REMOTE] = remote_splice_amnt;
- if (remote_splice_amnt < peer->channel->view[REMOTE].lowest_splice_amnt[LOCAL])
+ if (remote_splice_amnt < peer->channel->view[REMOTE].lowest_splice_amnt[REMOTE])
peer->channel->view[REMOTE].lowest_splice_amnt[REMOTE] = remote_splice_amnt;
}
}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.