Repository navigation
Port the v26.06.9 fixes to master - #9606
Conversation
json_getroutes() validated the maxparts pointer rather than its value, so maxparts=0 was forwarded to the solver. That empties the flow set and increase_flows() then calls amount_msat_div_ceil(shortage, tal_count(flows)) with a zero divisor, raising SIGFPE in the forked router child. Compare the value instead. Changelog-Fixed: JSON-RPC: `getroutes` with `maxparts=0` no longer crashes the askrene solver. (cherry picked from commit a9865a40f68888b1273dade5a504711cb422958e)
json_add_hex() used a variable-length array sized from the value it was formatting, so echoing a large datastore blob put ~2*len bytes on lightningd's stack and crashed it. Allocate the buffer with tal and hand it to json_add_string(), which frees it after copying. Changelog-Fixed: JSON-RPC: a very large `datastore` value no longer crashes lightningd. (cherry picked from commit ec58866dd6fd29708ba8588a1f6901335098fece)
…lose
An old commitment of ours that we revoked before the peer revoked its
own matching one has the number of the peer's current commitment, so
onchaind handles it as their unilateral close. None of its outputs
match there, and onchaind died:
STATUS_FAIL_INTERNAL_ERROR: Could not find resolution for output 2
As onchaind is restarted for the channel on every start, it died each
time and the channel stayed in ONCHAIN forever.
Log it as broken and carry on, as we already do for a revoked
commitment in handle_their_cheat, so the channel can resolve.
Changelog-Fixed: onchaind: a channel closed by one of our own old commitments no longer crashes onchaind on every restart, and now resolves.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit acaa225af64a2743426726417255fd6c5b40df54)
A source returning a "NaN"/"inf" JSON string price was accepted by parse::<f64>() and cached for SERVE_TTL. get_median then called partial_cmp(b).unwrap(), which panics on NaN; the panic ran in a tokio-spawned task, so the RPC reply was never sent and the caller's onion_message_recv hook stayed pending for each attempt. Reject non-finite prices when parsing, and sort with total_cmp so the median can never panic. Changelog-Fixed: currencyrate: reject non-finite prices from sources instead of hanging the RPC (cherry picked from commit 4f21a55e1101c61462efc87fb87b62923b58d63c)
The clnrest and wss-proxy plugins wrote their CA/server/client private keys using the default file mode, so with a permissive umask (e.g. 022) they ended up world-readable. The grpc-plugin already created key files with 0600; mirror that by creating the key files and setting mode 0600 before writing any contents. Changelog-Fixed: clnrest and wss-proxy: plugins now create TLS private keys with restrictive (0600) permissions regardless of umask. (cherry picked from commit d7466c6094b9ccebbc44d4f74ade658fa5616695)
tungstenite's default max_message_size is 64 MiB, and the accept loop had no bound on concurrent connections, so a peer could have us buffer up to 64 MiB per connection without limit. wss-proxy relays BOLT8 wire messages verbatim, which are at most 65535 bytes: set both max_message_size and max_frame_size to that bound so oversized messages are rejected instead of buffered. Bound concurrent connections with a semaphore held for the lifetime of each relay, and abort the peer relay once either direction closes so idle half-open connections cannot hold a slot (or a task) forever. Changelog-Fixed: wss-proxy: now rejects messages larger than a BOLT8 wire message (65535 bytes) and caps concurrent connections. (cherry picked from commit 8e620bb82772bec30698b465ea8c968ce9de2a78)
The TLS and websocket handshakes were awaited inline in the accept loop before per-connection work was spawned. A client which connected and never completed a handshake parked the loop, so no further accept() was serviced and the whole WSS endpoint was unavailable to everyone. Spawn a task per accepted stream immediately, and run both handshakes inside that task under a 10 second timeout (matching the pattern used by other plugins), so an idle client is dropped and can never block new connections. Changelog-Fixed: wss-proxy: an idle client no longer blocks the accept loop and prevents all new connections (cherry picked from commit cadf29066fa5927e99039d23401973ab73b93949)
The resolver client built on the proxy path already had a 30s timeout, but the default path used HTTPHrnResolver::new(), whose reqwest client has none: a server that accepts the connection and never replies left the plugin task, and the xpay command waiting on fetchbip353, pending forever. Build a single client with a 30s timeout for both paths, and wrap PaymentInstructions::parse in tokio::time::timeout so the whole resolution, not just each HTTP request, has a deadline. Changelog-Fixed: cln-bip353: fetchbip353 (and xpay payments to a human readable name) no longer hang when a remote endpoint stops responding (cherry picked from commit 885b70ab14b0353b231420717e44aae9a9288c46)
When an incoming onion doesn't parse we have no shared secret, and the HTLC can only be failed with update_fail_malformed_htlc. That's what happened when the parse failure was the reason, but an HTLC rejected first for another reason (dust exposure, the channel shutting down, or a blinded path turning every error into invalid_onion_blinding) took the onion-wrapped path, which needs the secret, and lightningd crashed. As the reject is replayed on restart, it crashed again on every start. Send malformed whenever there is no shared secret: with the parse failure code, or invalid_onion_blinding if the HTLC has a path_key. Changelog-Fixed: lightningd: an incoming HTLC whose onion can't be parsed is now always failed as malformed, instead of crashing the node in some cases. Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com> (cherry picked from commit d721a801312a376e34d5da5d7a5e516aed901f6e)
…ed past
On reestablish, a peer asking for our last commitment_signed again was
always given it, even when it had already sent the revoke_and_ack that
proves it received it. By then remote_per_commit has moved on to the
next commitment's point, so we signed that commitment number a second
time with the wrong per-commitment point, and only noticed afterwards:
Retransmitted commitment_signed 1 but they already send revocation 1?
Fail the channel instead, like any other bad next_commitment_number.
Splice resumption (next_funding) is unaffected: it legitimately asks
for that commitment after the revocation, and signs it with
old_remote_per_commit.
Changelog-Fixed: channeld: a peer's `channel_reestablish` asking for a commitment it has already acknowledged now fails the channel instead of getting it re-signed.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 516272a16b79746dbfaf49bcbe54ec239cdb462e)
htlcs_notify_new_block skipped the deadline force-close for any channel
that cannot add an HTLC, treating that as "already dying". A channel in
CHANNELD_SHUTTING_DOWN cannot add an HTLC but is still live in channeld
and can still remove one, so an offered HTLC that expired there was never
timed out on-chain. Instead, once the incoming HTLC neared its own
expiry, we failed it upstream, and a peer that fulfilled the outgoing
HTLC off-chain after that took the amount without us collecting it from
upstream ("FUNDS LOSS of ...: peer took funds onchain before we could
time out the HTLC, but we abandoned incoming HTLC").
Use the same test the fulfilled-HTLC loop below already uses: only skip
channels that are actually failing on-chain. Every other state where the
two predicates differ cannot hold an HTLC.
Changelog-Fixed: lightningd: an offered HTLC that reaches its deadline while the channel is shutting down now force-closes the channel, so forwarded funds cannot be lost to a late fulfill.
Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
(cherry picked from commit a5750b9bf6ac93a623a9d7823e9dfbbeefe7e32f)
peer_billboard function expects a format string and a variable number of arguments (like printf). Signed-off-by: Lagrang3 <lagrang3@protonmail.com> (cherry picked from commit 424a0b805569ad1cf00fd2b7153a4704bc2660fd)
handle_peer_splice_locked(), handle_splice_confirmed_signed(),
handle_splice_sending_sigs() and handle_update_inflight() call
channel_internal_error() when they can't find the inflight for a txid,
for example:
Unable to load inflight for locked_txid ...
channel_internal_error() fails the channel and returns, but they carried
on and dereferenced the NULL inflight, crashing lightningd instead of
just failing that channel. handle_update_inflight() likewise carried
on after "Must set last_tx and last_sig together", storing a half-set
pair.
Return after each of them. There's no test: reaching these needs
channeld to name an inflight lightningd doesn't have.
Changelog-Fixed: lightningd: a splice message for an inflight we don't know about now fails that channel instead of crashing the node.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
(cherry picked from commit 973a108622d5d33e2f159d20ff5db50157ab6fe0)
Changelog-Fixed: lightningd: `checkrune` no longer uses freed memory when a rune has several restrictions on the same parameter. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit bbd8855b3ef5cf21519a28b6671a6926c4cf0b41)
Changelog-Security: 'checkrune': 'method/createrune' and 'method/blacklistrune' restrictions can no longer be bypassed with the 'invokerune'/'destroyrune' aliases Changelog-Deprecated: JSON-RPC: 'invokerune' and 'destroyrune' (use 'createrune' and 'blacklistrune') Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 3f227cad487b093c5d6124af7be49a3919158370)
Changelog-Security: runes: a rune with restrictions can no longer relist blacklisted runes. Changelog-Changed: JSON-RPC: `blacklistrune` with `relist` needs a rune without restrictions when called through a rune, and refuses an `end` before `start`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit e81bc3695382149cfd03806dbf471b8ab034dc69)
By protocol: succesive reply_channel_range message must have first_blocknum equal to or greater than the previous first_blocknum. Changelog-Fixed: protocol: reply to query_channel_range with channels ranges in correct block order Signed-off-by: Lagrang3 <lagrang3@protonmail.com> (cherry picked from commit bbae1d9a94e671c5eead7899c3eeaf2726a22213)
…mmitment
channel_txs() returns NULL when it can't build a commitment, for example
when a splice amount would take one side's balance below zero:
channel_txs fails to amount_msat_add_sat_s64 ... LOCAL (side_pay + splice_amnt)
Neither send_commit_part() nor handle_peer_commit_sig() checked for that,
so channeld dereferenced NULL and crashed. Since every reconnect rebuilds
the same commitment, it then crashed again on each one, leaving the
channel stuck in a crash loop that only a unilateral close would end.
There's no way to continue on a commitment we can't build, so fail the
channel and say why. The next commit stops the case we know about
reaching here at all.
Changelog-None
Co-Authored-By: Ken Sedgwick <ken@bonsai.com>
(cherry picked from commit 2594edab350be11c68459e546921e6c0d1d07739)
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)
Every commitment the peer signs comes with its signatures for our HTLC transactions, and lightningd stores them so onchaind can spend the HTLC outputs if we close with that commitment. For a splice, got_commitsig stores them for each inflight on every later commitment update, but the inflight's first commitment, signed during the splice negotiation itself, goes to lightningd in channeld_update_inflight, which only carried the commitment and its signature. So any HTLC already pending when the splice was negotiated had no signature stored for the splice's commitment. That matters once the channel moves onto that commitment: after splice_locked, wallet_htlcsigs_confirm_inflight() promotes the inflight's stored sigs, which then lack those HTLCs, and the same goes for a close on a splice which confirmed before splice_locked. Add them to channeld_update_inflight, sent whenever it carries a new commitment, and have lightningd replace that inflight's stored sigs with them. channeld_update_inflight could be resent with the commitment lightningd already had when the user signs the splice; that now sends no commitment, so it can't replace those sigs with nothing. Changelog-None Co-Authored-By: Ken Sedgwick <ken@bonsai.com> (cherry picked from commit ebf69a8e6ba29560584060304bfe4a1e2b75dd52)
update_channel_from_inflight() moves channel->last_tx onto a splice's
commitment, both when the splice locks and when we adopt a splice which
confirmed before splice_locked because its output got spent. But
channel->last_htlc_sigs stayed the peer's signatures for the pre-splice
commitment until the next commitment update, or a restart, replaced them.
If that commitment is the one we close with first, onchaind gets
signatures which can't spend its HTLC outputs, so our HTLC-timeout
transactions are invalid and we can't get those HTLCs back. That's the
case in a report of a lost balance after a close on an unlocked splice:
the close broadcasts our inflight commitment (it spends the splice
funding output, because the splice had confirmed but never locked
in)
lightningd still treats the pre-splice output as the channel
funding, so onchaind is started with the pre-splice funding,
balance, and commitment
The sigs go with the commitment, so switch them in the same place:
wallet_htlcsigs_confirm_inflight() now returns the sigs it makes current,
and update_channel_from_inflight() calls it for a splice instead of
handle_peer_splice_locked() doing so on the side.
Reported-by: Ken Sedgwick <ken@bonsai.com>
Changelog-Fixed: splicing: closing with a splice's commitment before any later commitment update can now time out or claim the HTLCs on it.
Co-Authored-By: Ken Sedgwick <ken@bonsai.com>
(cherry picked from commit 275086a9b8eb36abe0dbb7499d7e5425a77e32ed)
Changelog-Security: runes: a rune with restrictions can no longer create a new rune without them. Changelog-Changed: JSON-RPC: `createrune` without a `rune` needs a rune without restrictions when called through a rune; any rune can still add restrictions to a rune it supplies. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 210b18a2a709d967e2e7d4fe218dd8779606eeb3)
Changelog-Fixed: lightningd: a valid rune missing from the database (e.g. after restoring an older backup) is recorded on first use, so its `per` and `rate` limits apply and its id isn't reused. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit a04ed5199856e7f0267ed13b69a1d242b1a151c9)
|
On FreeBSD 15.1 with clang: |
Changelog-Changed: JSON-RPC: `listconfigs` shows `...` instead of the value of `wallet`, `recover`, `tor-service-password`, `bitcoin-rpcpassword` and the old `bookkeeper-db`, for every caller; unset ones are omitted. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> (cherry picked from commit 6ed212e4856bce2b3392bac1d2fe2df90f3edfde)
An RBF that rebuilds a byte-identical splice transaction collides with the channel_funding_inflights primary key, and the DB layer treats that as fatal. Fail the pending splice command and the channel instead. Changelog-Fixed: A channel peer can no longer crash lightningd by RBF-ing a splice into a transaction the node has already recorded. Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit d268f127c6394578125fb3b7be9382548677f238)
Refuse the duplicate in channeld so the channel stays open, rather than leaving lightningd to fail the channel when it cannot record it. Changelog-None Co-authored-by: Cursor <cursoragent@cursor.com> (cherry picked from commit 9cb8764bb2f915a7e3bf3d2e4274a8de4e2c90ae)
Changelog-Fixed: `setconfig` no longer permits config-line injection through persistent option values. (cherry picked from commit 54178251aafc0ef793ab9a4dd9c44320bd40812d)
Changelog-Fixed: channeld: when accepting a splice, we now enforce the negotiated feerate on the splice transaction, so a peer can no longer get us to sign a splice paying (almost) no fees (cherry picked from commit f4d9b36c00bf67ccb4c4c0e929c3f63390001120)
The deprecation was written for a v26.09 release that is not happening. Start it with the next major release instead, keeping the same three releases until removal. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Copied from the v26.06.9 tag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
bd800b7 to
a003365
Compare
Thanks for testing on FreeBSD. This isn't caused by this PR. It comes from the ccan update that both master and v26.06.9 picked up. ccan's asort.h now declares qsort_r itself, and on FreeBSD 15 stdlib.h already declares it and wraps it in a macro, so the declaration clashes. The fix is up in ccan: rustyrussell/ccan#138. Once it's merged we'll bring it into master. Until then, for v26.06.9 on FreeBSD, this one-line guard in ccan/ccan/asort/asort.h gets it building: #if HAVE_QSORT_R_PRIVATE_LAST
+#ifndef qsort_r
void qsort_r(void *base, size_t nmemb, size_t size,
int (*compar)(const void *, const void *, void *), void *arg);
+#endif
#define _asort(b, n, s, cmp, ctx) qsort_r(b, n, s, cmp, ctx)Could you confirm it builds on 15.1 with that applied? |
Ah apologies. Should've filed that one upstream. This got me back up and running. Thank you! |
|
The CI that keeps failing seems to be due to memory exhaustion on the box. Merging now... |
This brings the fixes from the v26.06.9 point release forward to master: 28 commits cherry-picked from the fix branch they were written on, one follow-up moving a deprecation to v26.12, and one commit adding the 26.06.9 entry to
CHANGELOG.md. Each cherry-picked commit carries its(cherry picked from ...)line, and authorship is preserved throughout.These are the same fixes the release ships, source only. As announced in the release notes, the tests for the security fixes are temporarily withheld and will follow in a later PR. The fixes from the release that came from master in the first place (the runes bitmap over-read, the plugin JSON id escaping, the gossip throttle change from #9601, the release CI from #9602, and the other picks) are not repeated here.
What is in it
Channels, splicing and closing
channel_reestablishasking again for a commitment the peer has already acknowledged fails the channel instead of re-signing ittx_abort, and lightningd no longer aborts on a splice RBF reusing a funding txidcheck_balances()applyHTLCs, gossip and peers
query_channel_rangereplies keepfirst_blocknumnon-decreasing and every scid inside its reply's range, also when one block's channels span several repliespeer_billboardformat string fixRunes and configuration
checkrune: no use-after-free of the parameter cache keys;invokerune/destroyruneare checked ascreaterune/blacklistrune; only unrestricted runes may create new runes or relist blacklisted ones; a valid rune missing from the database is recorded on first uselistconfigsshows...forwallet,recover,tor-service-password,bitcoin-rpcpasswordandbookkeeper-dbsetconfigvalues containing a newline are rejectedPlugins and hardening
maxparts=0; hex decoding no longer allocates caller-sized buffers on the stack; currencyrate rejects non-finite pricesfetchbip353is bounded by HTTP and overall timeoutsNot a straight cherry-pick
lightningd: fix use-after-free of checkrune param cache keysis the original fix commit, not the squashed fix+test that landed on the fix branch, so itsChangelog-Fixedline is kept.setconfigfix) were picked without their test hunks.runes: start deprecating invokerune/destroyrune in v26.12: the alias deprecation was written for v26.09, which is not happening. It now starts in v26.12 and ends in v27.09 (the same three releases). The point release itself does not deprecate the aliases.Testing
Source checks and unit tests pass on this branch. The same fixes were tested on the v26.06.9 release tree (full unit and integration runs, vanilla, dual-fund and Liquid).
Written by Claude (AI assistant) on behalf of Níckolas.
🤖 Generated with Claude Code