From c5f199e40fee36f9ced7749dba307cccb1dc1dec Mon Sep 17 00:00:00 2001 From: Oliver Gugger Date: Mon, 20 Jul 2020 15:02:02 +0200 Subject: [PATCH] psbt: remove UTXO sanity check to allow fix for CVE As described in CVE-2020-14199 it is unsafe to only rely on witness UTXO information when signing. Hardware wallets fixed this by also requiring the full non-witness UTXO to be present for a witness input. To be compatible with those newer hardware wallet firmware, we need to remove the sanity checks that disallowed setting witness and non-witness UTXOs at the same time. See https://github.com/bitcoin/bitcoin/pull/19215 for comparison which removed the sanity checks in Bitcoin Core. --- psbt/partial_input.go | 16 +++++----------- psbt/psbt_test.go | 3 +++ 2 files changed, 8 insertions(+), 11 deletions(-) diff --git a/psbt/partial_input.go b/psbt/partial_input.go index 3db042f..4783c74 100644 --- a/psbt/partial_input.go +++ b/psbt/partial_input.go @@ -49,19 +49,13 @@ func NewPsbtInput(nonWitnessUtxo *wire.MsgTx, } // IsSane returns true only if there are no conflicting values in the Psbt -// PInput. It checks that witness and non-witness utxo entries do not both -// exist, and that witnessScript entries are only added to witness inputs. +// PInput. For segwit v0 no checks are currently implemented. func (pi *PInput) IsSane() bool { - if pi.NonWitnessUtxo != nil && pi.WitnessUtxo != nil { - return false - } - if pi.WitnessUtxo == nil && pi.WitnessScript != nil { - return false - } - if pi.WitnessUtxo == nil && pi.FinalScriptWitness != nil { - return false - } + // TODO(guggero): Implement sanity checks for segwit v1. For segwit v0 + // it is unsafe to only rely on the witness UTXO so we don't check that + // only one is set anymore. + // See https://github.com/bitcoin/bitcoin/pull/19215. return true } diff --git a/psbt/psbt_test.go b/psbt/psbt_test.go index 7029334..27f3b3f 100644 --- a/psbt/psbt_test.go +++ b/psbt/psbt_test.go @@ -161,6 +161,9 @@ func TestReadInvalidPsbt(t *testing.T) { } func TestSanityCheck(t *testing.T) { + // TODO(guggero): Remove when checks for segwit v1 are implemented. + t.Skip("Skipping PSBT sanity checks for segwit v0.") + // Test strategy: // 1. Create an invalid PSBT from a serialization // Then ensure that the sanity check fails.