Failure to reject `tx_add_output` amounts above the total bitcoin supply

[erickcestari]

BOLT 2 requires the receiver of tx_add_output to reject amounts above the total bitcoin supply:

The receiving node:

• MUST fail the negotiation if:
  • ...
  • the sats amount is greater than 2,100,000,000,000,000 (MAX_MONEY)

CLN does not implement this check. In both interactive-tx handlers, openingd/dualopend.c (run_tx_interactive, WIRE_TX_ADD_OUTPUT case) and common/interactivetx.c (process_interactivetx_updates, WIRE_TX_ADD_OUTPUT case), the sats field is converted with amount_sat() and passed straight to psbt_append_output(). The only checks before it are the message count, serial_id parity, duplicate serial_id, script type and output count.

libwally does enforce MAX_MONEY, and CLN asserts that libwally never fails. That turns a peer-controlled value into an abort of the subdaemon. There are two ways to hit it:

1. A single output above MAX_MONEY. wally_tx_output_init_alloc() refuses the amount, wally_tx_output() in bitcoin/tx.c returns NULL, and psbt_add_output() passes that NULL to wally_psbt_add_tx_output_at(), which returns WALLY_EINVAL. The assert at bitcoin/psbt.c:269 fires.
2. Several outputs that individually fit but sum above MAX_MONEY. Each one is accepted, since libwally only checks single values on construction. The first call to psbt_txid() after the negotiation (accepter_commits / opener_commits in dualopend, splice_accepter in channeld) runs wally_psbt_extract(), which rebuilds the transaction output by output. wally_tx_add_output() rejects the one that pushes the running total over the cap, and the assert at bitcoin/psbt.c:998 fires. This happens before check_balances() runs, and that function only guards against u64 overflow anyway, so reordering would not help.

This is the same class of bug as #4b34ad332 ("openingd: bound funding_satoshis by total bitcoin supply"), which capped open_channel2.funding_satoshis for exactly this reason. tx_add_output.sats is an independent wire field and was left unchecked.

Impact

• Dual funding (dualopend): any peer, with no channel and no funds, can crash dualopend in one round trip by opening a normally sized v2 channel and sending one tx_add_output with an oversized amount, or two funding outputs whose sum is oversized. lightningd logs the backtrace as BROKEN, deletes the unsaved channel and disconnects. Fresh node IDs are free, so this is repeatable at will. Requires --experimental-dual-fund.
• Splicing (channeld): an existing channel partner can send splice_init followed by the same tx_add_outputs and crash channeld for that channel. The channel is marked transiently failed and reconnects, after which the peer can repeat it. Pending HTLCs on that channel stall across each cycle. Splicing is enabled by default and inbound splice_init has no operator gate.

No funds are at risk. Channel state is persisted before the crash and no commitment_signed for the bad transaction is ever exchanged, so nothing invalid gets signed. HTLC deadline enforcement lives in lightningd (htlcs_notify_new_block) and force-closes without needing channeld, so a peer cannot use this to let an HTLC expire unclaimed. The effect is a remotely triggerable per-channel daemon crash and, for splicing, a per-channel availability attack by the partner.

Fix

Add a running-sum check at the tx_add_output site in both handlers, before psbt_append_output(): fail the negotiation if the existing PSBT output total plus the new sats exceeds chainparams->max_supply. A single output above the cap also pushes the total above it, so one check covers both cases. max_channel_funding() in openingd/common.c already documents the rationale.

Discovery

This bug was found while fuzzing the v2 funding protocol with smite.