diff --git a/rust/Cargo.lock b/rust/Cargo.lock index 6ecb7fd6af0..9fbf99adf40 100644 --- a/rust/Cargo.lock +++ b/rust/Cargo.lock @@ -1600,6 +1600,7 @@ dependencies = [ name = "tw_proto" version = "0.1.0" dependencies = [ + "arbitrary", "pb-rs", "quick-protobuf", "tw_encoding", diff --git a/rust/tw_evm/fuzz/Cargo.toml b/rust/tw_evm/fuzz/Cargo.toml index 78d9dacf87d..bc553ac5071 100644 --- a/rust/tw_evm/fuzz/Cargo.toml +++ b/rust/tw_evm/fuzz/Cargo.toml @@ -11,7 +11,7 @@ cargo-fuzz = true libfuzzer-sys = { version = "0.4", features = ["arbitrary-derive"] } serde_json = "1.0.95" tw_number = { path = "../../tw_number" } -tw_proto = { path = "../../tw_proto" } +tw_proto = { path = "../../tw_proto", features = ["fuzz"] } [dependencies.tw_evm] path = ".." @@ -47,3 +47,9 @@ name = "rlp_encode" path = "fuzz_targets/rlp_encode.rs" test = false doc = false + +[[bin]] +name = "sign" +path = "fuzz_targets/sign.rs" +test = false +doc = false diff --git a/rust/tw_evm/fuzz/fuzz_targets/sign.rs b/rust/tw_evm/fuzz/fuzz_targets/sign.rs new file mode 100644 index 00000000000..dfa541a2d4c --- /dev/null +++ b/rust/tw_evm/fuzz/fuzz_targets/sign.rs @@ -0,0 +1,16 @@ +// Copyright © 2017-2023 Trust Wallet. +// +// This file is part of Trust. The full Trust copyright notice, including +// terms governing use, modification, and redistribution, is contained in the +// file LICENSE at the root of the source code distribution tree. + +#![no_main] + +use libfuzzer_sys::fuzz_target; +use tw_evm::evm_context::StandardEvmContext; +use tw_evm::modules::signer::Signer; +use tw_proto::Ethereum::Proto; + +fuzz_target!(|input: Proto::SigningInput<'_>| { + let _ = Signer::::sign_proto(input); +}); diff --git a/rust/tw_evm/src/message/signature.rs b/rust/tw_evm/src/message/signature.rs index 7b006c05b74..847df7f4b4a 100644 --- a/rust/tw_evm/src/message/signature.rs +++ b/rust/tw_evm/src/message/signature.rs @@ -34,9 +34,12 @@ impl MessageSignature { pub fn prepared(sign: secp256k1::Signature, sign_type: SignatureType) -> KeyPairResult { let v = match sign_type { SignatureType::Standard => U256::from(sign.v()), - SignatureType::Legacy => legacy_replay_protection(sign.v()), + SignatureType::Legacy => { + legacy_replay_protection(sign.v()).map_err(|_| KeyPairError::InvalidSignature)? + }, SignatureType::Eip155 { chain_id } => { eip155_replay_protection(U256::from(chain_id), sign.v()) + .map_err(|_| KeyPairError::InvalidSignature)? }, }; Ok(MessageSignature { diff --git a/rust/tw_evm/src/modules/compiler.rs b/rust/tw_evm/src/modules/compiler.rs index 9210f3598bc..4d5066874b4 100644 --- a/rust/tw_evm/src/modules/compiler.rs +++ b/rust/tw_evm/src/modules/compiler.rs @@ -72,7 +72,7 @@ impl Compiler { let pre_hash = unsigned.pre_hash(chain_id); - let signed = unsigned.into_signed(signature, chain_id); + let signed = unsigned.try_into_signed(signature, chain_id)?; let eth_signature = signed.signature(); diff --git a/rust/tw_evm/src/modules/signer.rs b/rust/tw_evm/src/modules/signer.rs index 7353fb27227..18eb01b3374 100644 --- a/rust/tw_evm/src/modules/signer.rs +++ b/rust/tw_evm/src/modules/signer.rs @@ -39,7 +39,7 @@ impl Signer { let pre_hash = unsigned.pre_hash(chain_id); let signature = private_key.sign(pre_hash)?; - let signed = unsigned.into_signed(signature, chain_id); + let signed = unsigned.try_into_signed(signature, chain_id)?; let eth_signature = signed.signature(); diff --git a/rust/tw_evm/src/signature.rs b/rust/tw_evm/src/signature.rs index 31f3ab539d5..0949cb8027e 100644 --- a/rust/tw_evm/src/signature.rs +++ b/rust/tw_evm/src/signature.rs @@ -5,7 +5,7 @@ // file LICENSE at the root of the source code distribution tree. use std::ops::BitXor; -use tw_number::U256; +use tw_number::{NumberResult, U256}; /// EIP155 Eth encoding of V, of the form 27+v, or 35+chainID*2+v. /// cbindgin:ignore @@ -13,7 +13,7 @@ pub const ETHEREUM_SIGNATURE_V_OFFSET: u8 = 27; /// Embeds `chain_id` in `v` param, for replay protection, legacy or EIP155. #[inline] -pub fn replay_protection(chain_id: U256, v: u8) -> U256 { +pub fn replay_protection(chain_id: U256, v: u8) -> NumberResult { if chain_id.is_zero() { legacy_replay_protection(v) } else { @@ -23,14 +23,18 @@ pub fn replay_protection(chain_id: U256, v: u8) -> U256 { /// Embeds `chain_id` in `v` param, for replay protection, legacy. #[inline] -pub fn legacy_replay_protection(v: u8) -> U256 { - U256::from(v) + U256::from(ETHEREUM_SIGNATURE_V_OFFSET) +pub fn legacy_replay_protection(v: u8) -> NumberResult { + U256::from(v).checked_add(ETHEREUM_SIGNATURE_V_OFFSET) } /// Embeds `chain_id` in `v` param, for replay protection, EIP155. #[inline] -pub fn eip155_replay_protection(chain_id: U256, v: u8) -> U256 { - chain_id + chain_id + 35u8 + v +pub fn eip155_replay_protection(chain_id: U256, v: u8) -> NumberResult { + // chain_id + chain_id + 35u8 + v + chain_id + .checked_add(chain_id)? + .checked_add(35_u64)? + .checked_add(v) } /// Removes EIP155 or legacy replay protection. diff --git a/rust/tw_evm/src/transaction/mod.rs b/rust/tw_evm/src/transaction/mod.rs index dd9a9fa0fa9..a80521be11c 100644 --- a/rust/tw_evm/src/transaction/mod.rs +++ b/rust/tw_evm/src/transaction/mod.rs @@ -12,6 +12,7 @@ //! - User operations (EIP4337) use crate::transaction::signature::EthSignature; +use tw_coin_entry::error::SigningResult; use tw_hash::{sha3::keccak256, H256}; use tw_keypair::ecdsa::secp256k1; use tw_memory::Data; @@ -36,11 +37,11 @@ pub trait UnsignedTransaction: TransactionCommon { fn encode(&self, chain_id: U256) -> Data; - fn into_signed( + fn try_into_signed( self, signature: secp256k1::Signature, chain_id: U256, - ) -> Self::SignedTransaction; + ) -> SigningResult; } pub trait SignedTransaction: TransactionCommon { @@ -63,11 +64,11 @@ pub trait UnsignedTransactionBox: TransactionCommon { fn encode(&self, chain_id: U256) -> Data; - fn into_signed( + fn try_into_signed( self: Box, signature: secp256k1::Signature, chain_id: U256, - ) -> Box; + ) -> SigningResult>; } impl UnsignedTransactionBox for T @@ -82,14 +83,13 @@ where ::encode(self, chain_id) } - fn into_signed( + fn try_into_signed( self: Box, signature: secp256k1::Signature, chain_id: U256, - ) -> Box { - Box::new(::into_signed( - *self, signature, chain_id, - )) + ) -> SigningResult> { + let signed = ::try_into_signed(*self, signature, chain_id)?; + Ok(Box::new(signed)) } } diff --git a/rust/tw_evm/src/transaction/signature.rs b/rust/tw_evm/src/transaction/signature.rs index d0477b1d802..198ca2e91bb 100644 --- a/rust/tw_evm/src/transaction/signature.rs +++ b/rust/tw_evm/src/transaction/signature.rs @@ -7,6 +7,7 @@ use crate::signature::replay_protection; use tw_hash::H520; use tw_keypair::ecdsa::secp256k1; +use tw_keypair::{KeyPairError, KeyPairResult}; use tw_number::U256; pub trait EthSignature { @@ -68,13 +69,14 @@ pub struct SignatureEip155 { impl SignatureEip155 { #[inline] - pub fn new(sign: secp256k1::Signature, chain_id: U256) -> Self { - let v = replay_protection(chain_id, sign.v()); - SignatureEip155 { + pub fn new(sign: secp256k1::Signature, chain_id: U256) -> KeyPairResult { + let v = + replay_protection(chain_id, sign.v()).map_err(|_| KeyPairError::InvalidSignature)?; + Ok(SignatureEip155 { v, r: U256::from_big_endian(sign.r()), s: U256::from_big_endian(sign.s()), - } + }) } } @@ -126,7 +128,7 @@ mod tests { let secp_sign = secp256k1::Signature::from_bytes(secp_bytes.as_slice()).unwrap(); let chain_id = U256::zero(); - let eth_sign = SignatureEip155::new(secp_sign, chain_id); + let eth_sign = SignatureEip155::new(secp_sign, chain_id).unwrap(); let r = H256::from("d93fc9ae934d4f72db91cb149e7e84b50ca83b5a8a7b873b0fdb009546e3af47"); let s = H256::from("786bfaf31af61eea6471dbb1bec7d94f73fb90887e4f04d0e9b85676c47ab02a"); @@ -146,7 +148,7 @@ mod tests { let secp_sign = secp256k1::Signature::from_bytes(secp_bytes.as_slice()).unwrap(); let chain_id = U256::from(1_u64); - let eth_sign = SignatureEip155::new(secp_sign, chain_id); + let eth_sign = SignatureEip155::new(secp_sign, chain_id).unwrap(); let r = H256::from("d93fc9ae934d4f72db91cb149e7e84b50ca83b5a8a7b873b0fdb009546e3af47"); let s = H256::from("786bfaf31af61eea6471dbb1bec7d94f73fb90887e4f04d0e9b85676c47ab02a"); @@ -166,7 +168,7 @@ mod tests { let secp_sign = secp256k1::Signature::from_bytes(secp_bytes.as_slice()).unwrap(); let chain_id = U256::from(1_u64); - let eth_sign = SignatureEip155::new(secp_sign, chain_id); + let eth_sign = SignatureEip155::new(secp_sign, chain_id).unwrap(); let r = H256::from("42556c4f2a3f4e4e639cca524d1da70e60881417d4643e5382ed110a52719eaf"); let s = H256::from("172f591a2a763d0bd6b13d042d8c5eb66e87f129c9dc77ada66b6041012db2b3"); diff --git a/rust/tw_evm/src/transaction/transaction_eip1559.rs b/rust/tw_evm/src/transaction/transaction_eip1559.rs index 496e5a25a81..352a5b53a8e 100644 --- a/rust/tw_evm/src/transaction/transaction_eip1559.rs +++ b/rust/tw_evm/src/transaction/transaction_eip1559.rs @@ -8,6 +8,7 @@ use crate::address::Address; use crate::rlp::list::RlpList; use crate::transaction::signature::{EthSignature, Signature}; use crate::transaction::{SignedTransaction, TransactionCommon, UnsignedTransaction}; +use tw_coin_entry::error::SigningResult; use tw_keypair::ecdsa::secp256k1; use tw_memory::Data; use tw_number::U256; @@ -41,16 +42,16 @@ impl UnsignedTransaction for TransactionEip1559 { } #[inline] - fn into_signed( + fn try_into_signed( self, signature: secp256k1::Signature, chain_id: U256, - ) -> Self::SignedTransaction { - SignedTransactionEip1559 { + ) -> SigningResult { + Ok(SignedTransactionEip1559 { unsigned: self, signature: Signature::new(signature), chain_id, - } + }) } } diff --git a/rust/tw_evm/src/transaction/transaction_non_typed.rs b/rust/tw_evm/src/transaction/transaction_non_typed.rs index f5392539d16..67d386def60 100644 --- a/rust/tw_evm/src/transaction/transaction_non_typed.rs +++ b/rust/tw_evm/src/transaction/transaction_non_typed.rs @@ -8,6 +8,7 @@ use crate::address::Address; use crate::rlp::list::RlpList; use crate::transaction::signature::{EthSignature, SignatureEip155}; use crate::transaction::{SignedTransaction, TransactionCommon, UnsignedTransaction}; +use tw_coin_entry::error::SigningResult; use tw_keypair::ecdsa::secp256k1; use tw_memory::Data; use tw_number::U256; @@ -38,16 +39,16 @@ impl UnsignedTransaction for TransactionNonTyped { } #[inline] - fn into_signed( + fn try_into_signed( self, signature: secp256k1::Signature, chain_id: U256, - ) -> Self::SignedTransaction { - SignedTransactionNonTyped { + ) -> SigningResult { + Ok(SignedTransactionNonTyped { unsigned: self, - signature: SignatureEip155::new(signature, chain_id), + signature: SignatureEip155::new(signature, chain_id)?, chain_id, - } + }) } } diff --git a/rust/tw_evm/src/transaction/user_operation.rs b/rust/tw_evm/src/transaction/user_operation.rs index e2b0cf00e03..6abe6bd048b 100644 --- a/rust/tw_evm/src/transaction/user_operation.rs +++ b/rust/tw_evm/src/transaction/user_operation.rs @@ -11,6 +11,7 @@ use crate::address::Address; use crate::transaction::signature::Signature; use crate::transaction::{SignedTransaction, TransactionCommon, UnsignedTransaction}; use serde::Serialize; +use tw_coin_entry::error::SigningResult; use tw_encoding::hex; use tw_hash::sha3::keccak256; use tw_hash::H256; @@ -88,15 +89,15 @@ impl UnsignedTransaction for UserOperation { } #[inline] - fn into_signed( + fn try_into_signed( self, signature: tw_keypair::ecdsa::secp256k1::Signature, _chain_id: U256, - ) -> Self::SignedTransaction { - SignedUserOperation { + ) -> SigningResult { + Ok(SignedUserOperation { unsigned: self, signature: Signature::new(signature), - } + }) } } diff --git a/rust/tw_number/src/u256.rs b/rust/tw_number/src/u256.rs index 3bab3f1c82a..049fbb0b95b 100644 --- a/rust/tw_number/src/u256.rs +++ b/rust/tw_number/src/u256.rs @@ -129,6 +129,19 @@ impl U256 { self.0.byte(lowest_byte_idx) } + /// Checked addition. Returns `NumberError::IntegerOverflow` if overflow occurred. + #[inline] + pub fn checked_add(&self, rhs: T) -> NumberResult + where + T: Into, + { + let rhs = rhs.into(); + self.0 + .checked_add(rhs) + .map(U256) + .ok_or(NumberError::IntegerOverflow) + } + #[inline] fn leading_zero_bytes(&self) -> usize { U256::BYTES - (self.0.bits() + 7) / 8 diff --git a/rust/tw_proto/Cargo.toml b/rust/tw_proto/Cargo.toml index 22179b761bc..73be2eecd76 100644 --- a/rust/tw_proto/Cargo.toml +++ b/rust/tw_proto/Cargo.toml @@ -3,7 +3,12 @@ name = "tw_proto" version = "0.1.0" edition = "2021" +[features] +fuzz = ["arbitrary"] + [dependencies] +# Enable in fuzz tests only! +arbitrary = { version = "1", features = ["derive"], optional = true } quick-protobuf = "0.8.1" tw_encoding = { path = "../tw_encoding" } tw_memory = { path = "../tw_memory" } diff --git a/rust/tw_proto/build.rs b/rust/tw_proto/build.rs index 1ba71bd5a11..b3e60df9a68 100644 --- a/rust/tw_proto/build.rs +++ b/rust/tw_proto/build.rs @@ -6,6 +6,8 @@ use pb_rs::types::FileDescriptor; use pb_rs::ConfigBuilder; +#[cfg(feature = "fuzz")] +use std::io::{self, Read, Write}; use std::path::{Path, PathBuf}; use std::{env, fs}; @@ -50,4 +52,54 @@ fn main() { .expect("Error configuring pb-rs builder") .build(); FileDescriptor::run(&out_protos).expect("Error generating proto files"); + + #[cfg(feature = "fuzz")] + add_custom_derives(&out_dir, &["arbitrary::Arbitrary"]) + .expect("Error on adding 'arbitrary::Arbitrary' derive"); +} + +/// Unfortunately, `pb-rs` does not provide a proper support of custom derives. +/// [`ConfigBuilder::custom_struct_derive`] adds the derive macroses for structs only, +/// however they should be added for enums too. +/// Issues: https://github.com/tafia/quick-protobuf/issues/195, https://github.com/tafia/quick-protobuf/issues/212 +#[cfg(feature = "fuzz")] +fn add_custom_derives(out_dir: &Path, custom_derives: &[&str]) -> io::Result<()> { + // Debug is derived for all generated types. + let pattern = "#[derive(Debug"; + let replace_with = format!("#[derive(Debug, {}", custom_derives.join(", ")); + + let tw_dir = out_dir.join("TW"); + for blockchain_dir in tw_dir.read_dir()? { + let blockchain_dir = blockchain_dir?.path(); + + // There can be `mod.rs` files. Skip them. + if !blockchain_dir.is_dir() { + continue; + } + + let blockchain_proto = blockchain_dir.join("Proto.rs"); + replace_proto_content(&blockchain_proto, pattern, &replace_with)?; + } + + Ok(()) +} + +#[cfg(feature = "fuzz")] +fn replace_proto_content(path_to_file: &Path, pattern: &str, replace_with: &str) -> io::Result<()> { + let proto_content = { + let mut proto_file = fs::File::open(path_to_file)?; + + let mut proto_content = String::new(); + proto_file.read_to_string(&mut proto_content)?; + + proto_content + }; + + let upgraded_proto_content = proto_content.replace(pattern, &replace_with); + + let mut file = fs::OpenOptions::new() + .write(true) + .truncate(true) + .open(&path_to_file)?; + file.write_all(upgraded_proto_content.as_bytes()) }