Skip to content

psbt: sign segwit v0 inputs given only a prev tx - #552

Open
kkdao wants to merge 1 commit into
ElementsProject:masterfrom
kkdao:fix/segwit-non-witness-utxo
Open

kkdao wants to merge 1 commit into
ElementsProject:masterfrom
kkdao:fix/segwit-non-witness-utxo

Conversation

@kkdao

@kkdao kkdao commented Sep 30, 2026

Copy link
Copy Markdown

When an input carries a non_witness_utxo but no witness_utxo, the signature type is chosen
from the presence of witness_utxo alone, so a p2wpkh, p2sh-p2wpkh, p2wsh or p2sh-p2wsh output
backed only by the full previous transaction is signed with the legacy digest. Finalize and
extract then succeed, so the caller gets a complete transaction whose signature is invalid.

This change chooses the signature type from the referenced output's script, including a
matching P2SH redeem script, and checks the previous transaction's txid before using it.

The new test signs the same input with and without witness_utxo for all four script types,
verifies each signature against the BIP143 digest, requires the two signatures to be
identical, checks the serialized result and finalizes, and checks that a previous
transaction whose txid does not match the input is not signed from. It fails on master at
the BIP143 verification and passes with this change.

Validation, on master 374df23 plus this change alone (macOS, Apple clang 21, autoconf 2.73,
python 3.14.7):

  • ./configure --enable-debug --enable-export-all --disable-swig-python --disable-swig-java;
    make check: all 8 C test programs pass (test_bech32, test_psbt, test_psbt_limits,
    test_clear, test_coinselection, test_tx, test_descriptor, test_elements_tx).
  • All 32 Python ctypes test files pass, run directly.
  • The amalgamation compile check passes for all four BUILD_ARGS variants with -Werror (clang).
  • 3,000,000 mutated PSBTs (16 hand built shapes, three fixed seeds) run through master and
    this change under ASAN and UBSAN: 263,613 PSBTs give a different result, and every one of
    them contains an input backed only by a previous transaction whose referenced output is
    segwit v0; no PSBT without such an input changed; no sanitizer reports. This bounds where
    behaviour changed. Correctness of the changed signatures is established by the digest
    checks in the new test for its fixtures, not for every differing mutant.
    Not run here: valgrind, gcc ASAN/UBSAN, scan-build, cmake and mingw lanes. The no Elements
    ABI lane fails on this toolchain before and after the change, on unused static functions in
    libsecp256k1 headers under -Werror (for example src/util.h:34:13 print_buf_plain); with
    -Wno-unused-function added it builds and passes make check. That is an adapted local build,
    not the upstream lane.

When an input carries a non_witness_utxo but no witness_utxo, the
signature type was chosen from the presence of witness_utxo alone, so a
p2wpkh, p2sh-p2wpkh, p2wsh or p2sh-p2wsh output backed only by the full
previous transaction was signed with the legacy digest. Finalize and
extract then succeed, so the caller gets a complete transaction whose
signature is invalid.

Choose the signature type from the referenced output's script,
including a matching P2SH redeem script, and check the previous
transaction's txid before using it. Add a test that signs the same
input with and without witness_utxo for all four script types,
verifies each signature against the BIP143 digest and requires the two
signatures to be identical.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant