ElementsProject / ElementsProject/lightning

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

Open
#9,492 0 comments 2 reactions 1 assignee Claimed by @Lagrang3 View on GitHub
Dominant language
C
Stars
3.1k
Forks
1k
Avg merge
4d 10h
Merged PRs (30d)
13

Description

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_output`s 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](https://github.com/lnfuzz/smite).

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.