ElementsProject / lightning

Core Lightning — Lightning Network implementation focusing on spec compliance and performance
Other
2.85k stars 901 forks source link

Disallow funding PSBTs which use non-segwit inputs. #4500

Closed rustyrussell closed 1 year ago

rustyrussell commented 3 years ago

As described in https://lists.ozlabs.org/pipermail/c-lightning/2021-April/000200.html we should ensure that both normal funding PSBTs have no non-segwit inputs (for our own safety against PEBCAK) and insist there be no non-segwit inputs in the DF protocol (for safety against malicious peers).

Crypt-iQ commented 3 years ago

I haven't looked at the DF proposal, so I can't comment on it. But regarding the single-funder case, we were mostly concerned about a miner malleating a signature from low-S to high-S and changing the txid (since these inputs were not segwit inputs), making the commitment transactions exchanged during funding flow invalid.

rustyrussell commented 3 years ago

I haven't looked at the DF proposal, so I can't comment on it. But regarding the single-funder case, we were mostly concerned about a miner malleating a signature from low-S to high-S and changing the txid (since these inputs were not segwit inputs), making the commitment transactions exchanged during funding flow invalid.

Absolutely. But in the single funding case your mistake can only mess yourself up, with dual-funding you can mess up your peer!

niftynei commented 3 years ago

The dual-funding spec takes this into account/disallows it (and we've already got a check for it).

            /*
             * BOLT-f53ca2301232db780843e894f55d95d512f297f9 #2:
             * The receiving node: ...
             *   - MUST fail the negotiation if: ...
             *   - the `prevtx_out` input of `prevtx` is
             *   not an `OP_0` to `OP_16` followed by a single push
             */
            if (!is_segwit_output(&tx->wtx->outputs[outnum],
                          redeemscript))
                open_err_warn(state,
                          "Invalid tx sent. Not SegWit %s",
                          type_to_string(tmpctx,
                                 struct bitcoin_tx,
                                 tx));

If your peer sends you a non-segwit input, you fail the open.

niftynei commented 3 years ago

The only outstanding to do here would be for single-node funded PSBTs; it's a matter of adding an additional check to the new fundchannel_complete PSBT input.

If the tx was created elsewhere we have no way to verify its segwit-ness.