Skip to content

Commit

Permalink
[eth]: Add sign fuzz test
Browse files Browse the repository at this point in the history
* Add `arbitrary::Arbitrary` custom deriving for Proto files
* Fix overflow
  • Loading branch information
satoshiotomakan committed Oct 5, 2023
1 parent 1b7e03a commit 0e73344
Show file tree
Hide file tree
Showing 15 changed files with 144 additions and 39 deletions.
1 change: 1 addition & 0 deletions rust/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

8 changes: 7 additions & 1 deletion rust/tw_evm/fuzz/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ".."
Expand Down Expand Up @@ -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
16 changes: 16 additions & 0 deletions rust/tw_evm/fuzz/fuzz_targets/sign.rs
Original file line number Diff line number Diff line change
@@ -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::<StandardEvmContext>::sign_proto(input);
});
5 changes: 4 additions & 1 deletion rust/tw_evm/src/message/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,9 +34,12 @@ impl MessageSignature {
pub fn prepared(sign: secp256k1::Signature, sign_type: SignatureType) -> KeyPairResult<Self> {
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 {
Expand Down
2 changes: 1 addition & 1 deletion rust/tw_evm/src/modules/compiler.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ impl<Context: EvmContext> Compiler<Context> {

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();

Expand Down
2 changes: 1 addition & 1 deletion rust/tw_evm/src/modules/signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ impl<Context: EvmContext> Signer<Context> {
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();

Expand Down
16 changes: 10 additions & 6 deletions rust/tw_evm/src/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,15 @@
// 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
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<U256> {
if chain_id.is_zero() {
legacy_replay_protection(v)
} else {
Expand All @@ -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> {
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<U256> {
// 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.
Expand Down
18 changes: 9 additions & 9 deletions rust/tw_evm/src/transaction/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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<Self::SignedTransaction>;
}

pub trait SignedTransaction: TransactionCommon {
Expand All @@ -63,11 +64,11 @@ pub trait UnsignedTransactionBox: TransactionCommon {

fn encode(&self, chain_id: U256) -> Data;

fn into_signed(
fn try_into_signed(
self: Box<Self>,
signature: secp256k1::Signature,
chain_id: U256,
) -> Box<dyn SignedTransactionBox>;
) -> SigningResult<Box<dyn SignedTransactionBox>>;
}

impl<T> UnsignedTransactionBox for T
Expand All @@ -82,14 +83,13 @@ where
<Self as UnsignedTransaction>::encode(self, chain_id)
}

fn into_signed(
fn try_into_signed(
self: Box<Self>,
signature: secp256k1::Signature,
chain_id: U256,
) -> Box<dyn SignedTransactionBox> {
Box::new(<Self as UnsignedTransaction>::into_signed(
*self, signature, chain_id,
))
) -> SigningResult<Box<dyn SignedTransactionBox>> {
let signed = <Self as UnsignedTransaction>::try_into_signed(*self, signature, chain_id)?;
Ok(Box::new(signed))
}
}

Expand Down
16 changes: 9 additions & 7 deletions rust/tw_evm/src/transaction/signature.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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<Self> {
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()),
}
})
}
}

Expand Down Expand Up @@ -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");
Expand All @@ -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");
Expand All @@ -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");
Expand Down
9 changes: 5 additions & 4 deletions rust/tw_evm/src/transaction/transaction_eip1559.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Self::SignedTransaction> {
Ok(SignedTransactionEip1559 {
unsigned: self,
signature: Signature::new(signature),
chain_id,
}
})
}
}

Expand Down
11 changes: 6 additions & 5 deletions rust/tw_evm/src/transaction/transaction_non_typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Self::SignedTransaction> {
Ok(SignedTransactionNonTyped {
unsigned: self,
signature: SignatureEip155::new(signature, chain_id),
signature: SignatureEip155::new(signature, chain_id)?,
chain_id,
}
})
}
}

Expand Down
9 changes: 5 additions & 4 deletions rust/tw_evm/src/transaction/user_operation.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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<Self::SignedTransaction> {
Ok(SignedUserOperation {
unsigned: self,
signature: Signature::new(signature),
}
})
}
}

Expand Down
13 changes: 13 additions & 0 deletions rust/tw_number/src/u256.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T>(&self, rhs: T) -> NumberResult<U256>
where
T: Into<primitive_types::U256>,
{
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
Expand Down
5 changes: 5 additions & 0 deletions rust/tw_proto/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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" }
Expand Down
Loading

0 comments on commit 0e73344

Please sign in to comment.