fix(wasm-utxo): authenticate legacy PSBT prevout UTXO data - #413
Closed
ralph-bitgo[bot] wants to merge 1 commit into
Closed
ralph-bitgo[bot] wants to merge 1 commit into
ralph-bitgo[bot] wants to merge 1 commit into
Conversation
Legacy P2SH inputs on Bitcoin, Litecoin, Dogecoin, Dash and Pearl do not commit the input amount in their sighash, so a fabricated non_witness_utxo (or a lying witness_utxo) supplied by a compromised UTXO feed could inflate the displayed input value, pass the absurd-fee guard and hide a large miner fee behind fully valid signatures. - Check non_witness_utxo.compute_txid() against the prevout txid and cross-check witness_utxo against the referenced prevout output in get_output_script_and_value_for_network (Dash raw bytes and Zcash transparent txids follow network-specific rules and are validated separately). - Require the full previous transaction for legacy P2SH and p2shP2pk inputs on non-value-committing networks in add_wallet_input_to_psbt, add_replay_protection_input_to_psbt, add_input_at_index, BitGoPsbt::deserialize, hydration (HydrationUnspent gains an optional prevTx field) and the descriptor input builder. - Enforce the same validation before signing, parsing and fee-based extraction: sign, sign_with_privkey, sign_all_with_xpriv, musig2 context creation, parse_transaction_with_wallet_keys and the extract_tx* family. - Dash PSBT deserialization now verifies the preserved raw prevout bytes against the prevout txid. - Update AcidTest and fixed-script tests to build synthetic previous transactions whose computed txids match their outpoints, and add Rust and TypeScript regressions for txid mismatch, witness/non- witness disagreement, forged nested-segwit metadata and missing prevTx rejection. Ticket: WCN-1907 Session-Id: f3c6e0a8-c064-4f47-8c83-f52c665fe185 Task-Id: 078b961f-836b-4c49-a8ce-c08e0aacfc38 Requested-By: David Kaplan <davidkaplan@bitgo.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
get_output_script_and_value_for_network(new, inpsbt_wallet_input.rs) now authenticates PSBT input UTXO data:non_witness_utxo.compute_txid()must equal the prevout txid, andwitness_utxomust match the referencednon_witness_utxooutputwhen both are present. Dash and Zcash follow network-specific txid
rules and are validated separately.
transaction on networks whose sighash does not commit the input
amount (BTC/LTC/DOGE/DASH/Pearl, per
Network::requires_prev_tx_for_legacy_input), enforced inadd_wallet_input_to_psbt,add_replay_protection_input_to_psbt,add_input_at_index,BitGoPsbt::deserialize, hydration (HydrationUnspentgains anoptional
prevTxfield, wired through the JS/WASM boundary) and thedescriptor input builder (
add_input_with_descriptor).extraction:
sign,sign_with_privkey,sign_all_with_xpriv,sign_single_input_with_xpriv, musig2 context creation,parse_transaction_with_wallet_keys, and theextract_tx*family,so the absurd-fee guard and fee display consume authenticated values.
against the prevout txid (
compute_dash_txid).redeem_script/final_script_sig shape checks); forged metadata that
merely resembles nested segwit does not bypass the requirement.
psbt_wallet_input.rsanddash_psbt.rs;new TypeScript suite
test/fixedScript/legacyUtxoAuthentication.ts;AcidTest and the fixed-script suites now build synthetic previous
transactions whose computed txids match their outpoints.
Why
WCN-1907:
the legacy sighash commits only the scriptCode, not the input amount,
so a fabricated
non_witness_utxofrom a compromised UTXO feed couldinflate the displayed input value, pass the absurd-fee guard, and
produce valid signatures that burn the difference to miner fees.
BIP174 requires signers to verify the prevout txid; the repo's own CLI
already did (
cli/src/psbt/add_input.rs), but no library signing pathdid. Value-committing networks (Zcash ZIP-243, BCH-family FORKID,
segwit, taproot) are unaffected and keep witness-only inputs.
Test plan
(
test/fixtures/fixed-script/,cli/test/fixtures/...,packages/webui/src/fixtures/...) — their synthetic prevoutspredate this change and now fail txid authentication.
cd packages/wasm-utxo && cargo test(newprevout_authentication_tests,test_dash_psbt_*, and existingsuites).
npm testinpackages/wasm-utxo(newlegacyUtxoAuthentication.ts+ updated fixedScript suites).Note: this sandbox had no Rust/Node toolchain, so the diff is
unverified: it compiles unproven and fixtures are not yet regenerated.
Draft is pushed for toolchain-backed CI and follow-up fixture
regeneration.
Ticket: WCN-1907