diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 6db5c3df2e..69a624ee6f 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -2664,6 +2664,7 @@ dependencies = [ "tw_ton", "tw_ton_sdk", "tw_utxo", + "tw_zcash", "wallet-core-rs", ] diff --git a/rust/chains/tw_bitcoin/src/modules/compiler.rs b/rust/chains/tw_bitcoin/src/modules/compiler.rs index f6e4a5fa93..91ffa266e1 100644 --- a/rust/chains/tw_bitcoin/src/modules/compiler.rs +++ b/rust/chains/tw_bitcoin/src/modules/compiler.rs @@ -53,7 +53,7 @@ impl BitcoinCompiler { TxPlanner::plan(request)?.unsigned_tx }, TransactionType::psbt(ref psbt) => { - Context::PsbtRequestHandler::parse_request(&input, psbt)?.unsigned_tx + Context::PsbtRequestHandler::parse_request(coin, &input, psbt)?.unsigned_tx }, TransactionType::None => { return SigningError::err(SigningErrorType::Error_invalid_params) @@ -132,13 +132,13 @@ impl BitcoinCompiler { } fn compile_psbt( - _coin: &dyn CoinContext, + coin: &dyn CoinContext, input: &Proto::SigningInput, psbt: &Proto::Psbt, signatures: Vec, ) -> SigningResult> { let PsbtRequest { unsigned_tx, .. } = - Context::PsbtRequestHandler::parse_request(input, psbt)?; + Context::PsbtRequestHandler::parse_request(coin, input, psbt)?; let fee = unsigned_tx.fee()?; SighashVerifier::verify_signatures(&unsigned_tx, &signatures)?; diff --git a/rust/chains/tw_bitcoin/src/modules/planner/psbt_planner.rs b/rust/chains/tw_bitcoin/src/modules/planner/psbt_planner.rs index 8aa40f5858..724e46a854 100644 --- a/rust/chains/tw_bitcoin/src/modules/planner/psbt_planner.rs +++ b/rust/chains/tw_bitcoin/src/modules/planner/psbt_planner.rs @@ -32,7 +32,7 @@ impl PsbtPlanner { ) -> SigningResult> { let chain_info = chain_info(coin, &input.chain_info)?; let PsbtRequest { unsigned_tx, .. } = - Context::PsbtRequestHandler::parse_request(input, psbt_input)?; + Context::PsbtRequestHandler::parse_request(coin, input, psbt_input)?; let total_input = unsigned_tx.total_input()?; let fee_estimate = unsigned_tx.fee()?; diff --git a/rust/chains/tw_bitcoin/src/modules/psbt_request/mod.rs b/rust/chains/tw_bitcoin/src/modules/psbt_request/mod.rs index df3161df34..35dae1a054 100644 --- a/rust/chains/tw_bitcoin/src/modules/psbt_request/mod.rs +++ b/rust/chains/tw_bitcoin/src/modules/psbt_request/mod.rs @@ -3,6 +3,7 @@ // Copyright © 2017 Trust Wallet. use std::marker::PhantomData; +use tw_coin_entry::coin_context::CoinContext; use tw_coin_entry::error::prelude::*; use tw_memory::Data; use tw_proto::BitcoinV2::Proto; @@ -18,6 +19,7 @@ pub mod utxo_psbt; pub trait PsbtRequestHandler { /// Parses a PSBT request from Protobuf. fn parse_request( + coin: &dyn CoinContext, input: &Proto::SigningInput, psbt_input: &Proto::Psbt, ) -> SigningResult>; @@ -43,6 +45,7 @@ pub struct NoPsbtRequestBuilder; impl PsbtRequestHandler for NoPsbtRequestBuilder { fn parse_request( + _coin: &dyn CoinContext, _input: &Proto::SigningInput, _psbt_input: &Proto::Psbt, ) -> SigningResult> { @@ -77,8 +80,10 @@ mod tests { let psbt_input = Proto::Psbt::default(); // Test parse_request + use tw_coin_entry::test_utils::test_context::TestCoinContext; + let coin = TestCoinContext::default(); let result: SigningResult> = - NoPsbtRequestBuilder::parse_request(&input, &psbt_input); + NoPsbtRequestBuilder::parse_request(&coin, &input, &psbt_input); assert_eq!( *result.err().expect("Expected an error").error_type(), SigningErrorType::Error_not_supported diff --git a/rust/chains/tw_bitcoin/src/modules/psbt_request/standard_psbt_request_handler.rs b/rust/chains/tw_bitcoin/src/modules/psbt_request/standard_psbt_request_handler.rs index 99f24d029e..de7c8b71f8 100644 --- a/rust/chains/tw_bitcoin/src/modules/psbt_request/standard_psbt_request_handler.rs +++ b/rust/chains/tw_bitcoin/src/modules/psbt_request/standard_psbt_request_handler.rs @@ -7,6 +7,7 @@ use crate::modules::psbt_request::utxo_psbt::UtxoPsbt; use crate::modules::psbt_request::{PsbtRequest, PsbtRequestHandler}; use crate::modules::signing_request::standard_signing_request::StandardSigningRequestBuilder; use std::marker::PhantomData; +use tw_coin_entry::coin_context::CoinContext; use tw_coin_entry::error::prelude::*; use tw_memory::Data; use tw_proto::BitcoinV2::Proto; @@ -24,6 +25,7 @@ where Context: UtxoContext, { fn parse_request( + _coin: &dyn CoinContext, input: &Proto::SigningInput, psbt_input: &Proto::Psbt, ) -> SigningResult> { diff --git a/rust/chains/tw_bitcoin/src/modules/signer.rs b/rust/chains/tw_bitcoin/src/modules/signer.rs index 9ee8709690..9f455901ea 100644 --- a/rust/chains/tw_bitcoin/src/modules/signer.rs +++ b/rust/chains/tw_bitcoin/src/modules/signer.rs @@ -80,7 +80,7 @@ impl BitcoinSigner { } pub fn sign_psbt( - _coin: &dyn CoinContext, + coin: &dyn CoinContext, input: &Proto::SigningInput, psbt_input: &Proto::Psbt, ) -> SigningResult> { @@ -88,7 +88,7 @@ impl BitcoinSigner { mut psbt, unsigned_tx, .. - } = Context::PsbtRequestHandler::parse_request(input, psbt_input)?; + } = Context::PsbtRequestHandler::parse_request(coin, input, psbt_input)?; let fee = unsigned_tx.fee()?; diff --git a/rust/chains/tw_zcash/src/modules/pczt_request/output_pczt.rs b/rust/chains/tw_zcash/src/modules/pczt_request/output_pczt.rs index 480d7425c9..087e04a69b 100644 --- a/rust/chains/tw_zcash/src/modules/pczt_request/output_pczt.rs +++ b/rust/chains/tw_zcash/src/modules/pczt_request/output_pczt.rs @@ -2,19 +2,25 @@ // // Copyright © 2017 Trust Wallet. +use crate::address::ZcashAddress; +use crate::context::ZcashContext; use crate::modules::pczt; +use std::str::FromStr; +use tw_bitcoin::modules::tx_builder::BitcoinChainInfo; use tw_coin_entry::error::prelude::{MapTWError, ResultContext, SigningErrorType, SigningResult}; +use tw_utxo::context::UtxoContext; use tw_utxo::script::Script; use tw_utxo::transaction::standard_transaction::TransactionOutput; /// Currently, we rely on `pczt` crate to build our own [`TransactionOutput`]. pub struct OutputPczt<'a> { output: &'a pczt::transparent::Output, + chain_info: &'a BitcoinChainInfo, } impl<'a> OutputPczt<'a> { - pub fn new(output: &'a pczt::transparent::Output) -> Self { - OutputPczt { output } + pub fn new(output: &'a pczt::transparent::Output, chain_info: &'a BitcoinChainInfo) -> Self { + OutputPczt { output, chain_info } } pub fn build(self) -> SigningResult { @@ -25,9 +31,99 @@ impl<'a> OutputPczt<'a> { .tw_err(SigningErrorType::Error_invalid_requested_token_amount) .context("PCZT Output amount is too large")?; let script_pubkey = Script::from(self.output.script_pubkey.clone()); + + if let Some(ref addr_str) = self.output.user_address { + let addr = ZcashAddress::from_str(addr_str) + .tw_err(SigningErrorType::Error_invalid_address) + .context("PCZT Output user_address is not a valid Zcash address")?; + let expected_script = + ZcashContext::addr_to_script_pubkey(&addr, self.chain_info.to_address_prefixes()) + .context("PCZT Output user_address cannot be converted to a script")?; + if expected_script != script_pubkey { + return tw_coin_entry::error::prelude::SigningError::err( + SigningErrorType::Error_invalid_address, + ) + .context("PCZT Output user_address does not match script_pubkey"); + } + } + Ok(TransactionOutput { value, script_pubkey, }) } } + +#[cfg(test)] +mod tests { + use super::*; + use std::collections::BTreeMap; + + fn chain_info() -> BitcoinChainInfo { + BitcoinChainInfo { + p2pkh_prefix: 0xB8, + p2sh_prefix: 0xBD, + hrp: None, + } + } + + fn make_output( + script_pubkey: Vec, + user_address: Option<&str>, + ) -> pczt::transparent::Output { + pczt::transparent::Output { + value: 100_000, + script_pubkey, + redeem_script: None, + bip32_derivation: BTreeMap::new(), + user_address: user_address.map(str::to_owned), + proprietary: BTreeMap::new(), + } + } + + const ATTACKER_SCRIPT: &str = "76a91449964a736f3713d64283fd0018626ba50091c7e988ac"; + + const MERCHANT_ADDR: &str = "t1gWVE2uyrET2CxSmCaBiKzmWxQdHhnvMSz"; + const MERCHANT_SCRIPT: &str = "76a914f84c7f4dd3c3dc311676444fdead6e6d290d50e388ac"; + + fn hex_to_bytes(hex: &str) -> Vec { + (0..hex.len()) + .step_by(2) + .map(|i| u8::from_str_radix(&hex[i..i + 2], 16).unwrap()) + .collect() + } + + #[test] + fn test_no_user_address_passes() { + let info = chain_info(); + let output = make_output(hex_to_bytes(MERCHANT_SCRIPT), None); + assert!(OutputPczt::new(&output, &info).build().is_ok()); + } + + #[test] + fn test_matching_user_address_passes() { + let info = chain_info(); + let output = make_output(hex_to_bytes(MERCHANT_SCRIPT), Some(MERCHANT_ADDR)); + assert!(OutputPczt::new(&output, &info).build().is_ok()); + } + + #[test] + fn test_mismatched_user_address_rejected() { + let info = chain_info(); + let output = make_output(hex_to_bytes(ATTACKER_SCRIPT), Some(MERCHANT_ADDR)); + let result = OutputPczt::new(&output, &info).build(); + assert!(result.is_err()); + assert_eq!( + *result.unwrap_err().error_type(), + tw_coin_entry::error::prelude::SigningErrorType::Error_invalid_address, + ); + } + + #[test] + fn test_invalid_user_address_rejected() { + let info = chain_info(); + let output = make_output(hex_to_bytes(MERCHANT_SCRIPT), Some("not_a_valid_address")); + let result = OutputPczt::new(&output, &info).build(); + assert!(result.is_err()); + } +} diff --git a/rust/chains/tw_zcash/src/modules/pczt_request/zcash_pczt_request_handler.rs b/rust/chains/tw_zcash/src/modules/pczt_request/zcash_pczt_request_handler.rs index dcce2b30bd..fcbc17401c 100644 --- a/rust/chains/tw_zcash/src/modules/pczt_request/zcash_pczt_request_handler.rs +++ b/rust/chains/tw_zcash/src/modules/pczt_request/zcash_pczt_request_handler.rs @@ -9,7 +9,10 @@ use crate::modules::transaction_builder::ZcashTransactionBuilder; use crate::transaction::{ZcashTransaction, TRANSACTION_VERSION_4, TRANSACTION_VERSION_GROUP_ID}; use std::marker::PhantomData; use tw_bitcoin::modules::psbt_request::{PsbtRequest, PsbtRequestHandler}; -use tw_bitcoin::modules::signing_request::standard_signing_request::StandardSigningRequestBuilder; +use tw_bitcoin::modules::signing_request::standard_signing_request::{ + chain_info, StandardSigningRequestBuilder, +}; +use tw_coin_entry::coin_context::CoinContext; use tw_coin_entry::error::prelude::{ResultContext, SigningError, SigningErrorType, SigningResult}; use tw_hash::H32; use tw_memory::Data; @@ -24,9 +27,11 @@ where Context: UtxoContext, { fn parse_request( + coin: &dyn CoinContext, input: &Proto::SigningInput, psbt_input: &Proto::Psbt, ) -> SigningResult> { + let chain_info = chain_info(coin, &input.chain_info)?; let pczt = pczt::Pczt::deserialize(&psbt_input.psbt)?; let version = pczt.global.tx_version; @@ -76,7 +81,7 @@ where // Add all outputs to the unsigned transaction builder. for txout in pczt.transparent.outputs.iter() { - let output = OutputPczt::new(txout) + let output = OutputPczt::new(txout, &chain_info) .build() .context("Error creating Output from PCZT")?; builder.push_output(output); diff --git a/rust/tw_tests/Cargo.toml b/rust/tw_tests/Cargo.toml index 97068fbdd9..f612847fa2 100644 --- a/rust/tw_tests/Cargo.toml +++ b/rust/tw_tests/Cargo.toml @@ -24,3 +24,4 @@ tw_solana = { path = "../chains/tw_solana" } tw_ton = { path = "../chains/tw_ton" } tw_ton_sdk = { path = "../frameworks/tw_ton_sdk" } tw_utxo = { path = "../frameworks/tw_utxo" } +tw_zcash = { path = "../chains/tw_zcash" } diff --git a/rust/tw_tests/tests/chains/zcash/zcash_sign.rs b/rust/tw_tests/tests/chains/zcash/zcash_sign.rs index fea83a3606..f4749f6cbb 100644 --- a/rust/tw_tests/tests/chains/zcash/zcash_sign.rs +++ b/rust/tw_tests/tests/chains/zcash/zcash_sign.rs @@ -9,10 +9,12 @@ use crate::chains::common::bitcoin::{ use crate::chains::zcash::{zcash_extra_data, zec_info, NU6_BRANCH_ID, SAPLING_BRANCH_ID}; use tw_any_coin::test_utils::sign_utils::AnySignerHelper; use tw_coin_registry::coin_type::CoinType; +use tw_encoding::base64::{self, STANDARD}; use tw_encoding::hex::DecodeHex; use tw_proto::BitcoinV2::Proto; use tw_proto::Common::Proto::SigningError; use tw_proto::Zcash::Proto as ZcashProto; +use tw_zcash::modules::pczt::Pczt; /// Successfully broadcasted: /// https://explorer.zcha.in/transactions/ec9033381c1cc53ada837ef9981c03ead1c7c41700ff3a954389cfaddc949256 @@ -314,3 +316,45 @@ fn test_zcash_sign_pczt_unsupported_tx_version() { let output = signer.sign(CoinType::Zcash, signing); assert_eq!(output.error, SigningError::Error_not_supported); } + +/// Builds a modified PCZT from the base PCZT used in `test_zcash_sign_pczt`, +/// setting `user_address` on the first output. +fn pczt_with_user_address(user_address: Option<&str>) -> String { + const BASE_PSBT: &str = "UENaVAEAAAAEhcG8yQjVoJzHDAEAz+i/AYUBgwACD/xF9t6uGgnrn6HPk4TTxoTB42mxjfZ2E8dQ7UzfodcAAf////8PAAAAgIl6GXapFFWjZeeDsUiGjDI8hm1k+WCt0MtqiKwAAAEAAAAAAACg+XQ96jWHJsJWFA3/kk+6ZaHhUI5wi57Hf3ZM6xQzRQEB/////w8AAAC1lhgZdqkUVaNl54OxSIaMMjyGbWT5YK3Qy2qIrAAAAQAAAAAAAALD8X4ZdqkUkFFaBL/xtThv5Hnc0F1hHGFf2sSIrAAAASN0MVgyZ21OUVJxd1dpR2luY0syVXRZd1piV2c3aXhZRkVLcgDCwxEZdqkUVaNl54OxSIaMMjyGbWT5YK3Qy2qIrAAAASN0MVJnUkJqam54WFNnMXB0TERya2FNTmlWNHRKVlh1N2RXVgAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAwABAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=="; + let bytes = base64::decode(BASE_PSBT, STANDARD).unwrap(); + let mut pczt = Pczt::deserialize(&bytes).unwrap(); + pczt.transparent.outputs[0].user_address = user_address.map(str::to_owned); + let serialized = pczt.serialize().unwrap(); + base64::encode(&serialized, STANDARD) +} + +fn sign_pczt_with_user_address(user_address: &str) -> SigningError { + const PRIVATE_KEY: &str = "c9d84f11d992c1a527293b468ba67f739f8098c333748493da45b9cf53844ec4"; + let psbt = pczt_with_user_address(Some(user_address)); + + let signing = Proto::SigningInput { + private_keys: vec![PRIVATE_KEY.decode_hex().unwrap().into()], + chain_info: zec_info(), + transaction: transaction_psbt_b64(&psbt), + ..Default::default() + }; + + let mut signer = AnySignerHelper::::default(); + signer.sign(CoinType::Zcash, signing).error +} + +#[test] +fn test_zcash_sign_pczt_matching_user_address() { + assert_eq!( + sign_pczt_with_user_address("t1X2gmNQRqwWiGincK2UtYwZbWg7ixYFEKr"), + SigningError::OK + ); +} + +#[test] +fn test_zcash_sign_pczt_mismatched_user_address_rejected() { + assert_eq!( + sign_pczt_with_user_address("t1gWVE2uyrET2CxSmCaBiKzmWxQdHhnvMSz"), + SigningError::Error_invalid_address + ); +}