From 72c67becb5f22ca3d6492cc7577aa3b8911ad7cb Mon Sep 17 00:00:00 2001 From: satoshiotomakan <127754187+satoshiotomakan@users.noreply.github.com> Date: Tue, 31 Oct 2023 11:04:53 +0100 Subject: [PATCH] [Eth]: Fix EIP712 message hashing when fixed byte array is `0x0` (#3522) --- rust/tw_encoding/src/hex.rs | 23 ++++- .../src/message/eip712/eip712_message.rs | 9 +- .../tw_evm/tests/data/eip712_fixed_bytes.json | 83 +++++++++++++++++++ rust/tw_evm/tests/message_signer.rs | 13 +++ 4 files changed, 120 insertions(+), 8 deletions(-) create mode 100644 rust/tw_evm/tests/data/eip712_fixed_bytes.json diff --git a/rust/tw_encoding/src/hex.rs b/rust/tw_encoding/src/hex.rs index d7cb5a5da95..38dc8b2e44f 100644 --- a/rust/tw_encoding/src/hex.rs +++ b/rust/tw_encoding/src/hex.rs @@ -5,6 +5,9 @@ // file LICENSE at the root of the source code distribution tree. pub use hex::FromHexError; +use tw_memory::Data; + +pub type FromHexResult = Result; pub trait ToHex { fn to_hex(&self) -> String; @@ -26,20 +29,34 @@ where } pub trait DecodeHex { - fn decode_hex(&self) -> Result, FromHexError>; + fn decode_hex(&self) -> FromHexResult; } impl<'a> DecodeHex for &'a str { - fn decode_hex(&self) -> Result, FromHexError> { + fn decode_hex(&self) -> FromHexResult { decode(self) } } -pub fn decode(data: &str) -> Result, FromHexError> { +/// Decodes the given hexadecimal string. +pub fn decode(data: &str) -> FromHexResult { let hex_string = data.trim_start_matches("0x"); hex::decode(hex_string) } +/// Decodes the given hexadecimal string leniently allowing to pass odd number of chars. +/// For example, `0x0` is extended to `0x00`, `0x123` is extended to `0x0123`. +pub fn decode_lenient(data: &str) -> FromHexResult { + let hex_string = data.trim_start_matches("0x"); + if hex_string.len() % 2 == 0 { + hex::decode(hex_string) + } else { + // Insert a leading 0. + let standard_hex = format!("0{hex_string}"); + hex::decode(standard_hex) + } +} + pub fn encode>(data: T, prefixed: bool) -> String { let encoded = hex::encode(data.as_ref()); if prefixed { diff --git a/rust/tw_evm/src/message/eip712/eip712_message.rs b/rust/tw_evm/src/message/eip712/eip712_message.rs index 7be57299689..9f21b15efaa 100644 --- a/rust/tw_evm/src/message/eip712/eip712_message.rs +++ b/rust/tw_evm/src/message/eip712/eip712_message.rs @@ -15,7 +15,7 @@ use serde::Deserialize; use serde_json::Value as Json; use std::collections::HashMap; use std::str::FromStr; -use tw_encoding::hex::DecodeHex; +use tw_encoding::hex::{self, DecodeHex}; use tw_hash::sha3::keccak256; use tw_hash::{H160, H256}; use tw_memory::Data; @@ -158,10 +158,9 @@ fn encode_fix_bytes(value: &Json, expected_len: usize) -> MessageSigningResult expected_len { return Err(MessageSigningError::TypeValueMismatch); } let checked_bytes = diff --git a/rust/tw_evm/tests/data/eip712_fixed_bytes.json b/rust/tw_evm/tests/data/eip712_fixed_bytes.json new file mode 100644 index 00000000000..563c8eaaa37 --- /dev/null +++ b/rust/tw_evm/tests/data/eip712_fixed_bytes.json @@ -0,0 +1,83 @@ +{ + "types": { + "EIP712Domain": [ + { + "name": "name", + "type": "string" + }, + { + "name": "version", + "type": "string" + }, + { + "name": "chainId", + "type": "uint256" + }, + { + "name": "verifyingContract", + "type": "address" + }, + { + "name": "salt", + "type": "bytes32" + } + ], + "Trade": [ + { + "name": "nonce", + "type": "bytes32" + }, + { + "name": "firstParty", + "type": "address" + }, + { + "name": "askingId", + "type": "uint256" + }, + { + "name": "askingQty", + "type": "uint256" + }, + { + "name": "offeringId", + "type": "uint256" + }, + { + "name": "offeringQty", + "type": "uint256" + }, + { + "name": "maxFee", + "type": "uint256" + }, + { + "name": "secondParty", + "type": "address" + }, + { + "name": "count", + "type": "uint8" + } + ] + }, + "domain": { + "name": "CryptoFights Trading", + "version": "1", + "chainId": 1, + "verifyingContract": "0xdc45529aC0FA3185f79A005e57deF64F600c4e97", + "salt": "0x0" + }, + "primaryType": "Trade", + "message": { + "count": 1, + "offeringQty": 1, + "askingQty": 2, + "nonce": "0xcfe49aa546453df3f2e768a97204a3268cef7c27df53cc2f2d47385cfeaf", + "firstParty": "0xC36edF48e21cf395B206352A1819DE658fD7f988", + "askingId": "0x0000000000000000000000000000000000000000000000000000000000000000", + "offeringId": "0x0000000000000000000000000000000000000000000000000000000000000000", + "maxFee": "1000000000000000000", + "secondParty": "0x0000000000000000000000000000000000000000" + } +} \ No newline at end of file diff --git a/rust/tw_evm/tests/message_signer.rs b/rust/tw_evm/tests/message_signer.rs index a1491c61dcd..7be25764c2a 100644 --- a/rust/tw_evm/tests/message_signer.rs +++ b/rust/tw_evm/tests/message_signer.rs @@ -19,6 +19,7 @@ const EIP712_WITH_CUSTOM_ARRAY: &str = include_str!("data/eip712_with_custom_arr const EIP712_UNEQUAL_ARRAY_LEN: &str = include_str!("data/eip712_unequal_array_lengths.json"); const EIP712_WITH_CHAIN_ID_STR: &str = include_str!("data/eip712_with_chain_id_string.json"); const EIP712_GREENFIELD: &str = include_str!("data/eip712_greenfield.json"); +const EIP712_FIXED_BYTES: &str = include_str!("data/eip712_fixed_bytes.json"); struct SignVerifyTestInput { private_key: &'static str, @@ -256,3 +257,15 @@ fn test_message_signer_sign_verify_eip712_greenfield() { signature: "cb3a4684a991014a387a04a85b59227ebb79567c2025addcb296b4ca856e9f810d3b526f2a0d0fad6ad1b126b3b9516f8b3be020a7cca9c03ce3cf47f4199b6d1b", }); } + +// The test checks if `0x0` is a valid `bytes32` value. +#[test] +fn test_message_signer_sign_verify_eip712_fixed_bytes() { + test_message_signer_sign_verify(SignVerifyTestInput { + private_key: "c85ef7d79691fe79573b1a7064c19c1a9819ebdbd1faaab1a8ec92344438aaf4", + msg: EIP712_FIXED_BYTES, + msg_type: Proto::MessageType::MessageType_typed, + chain_id: None, + signature: "7ee9b54fedf355e40fa86bbe23e63b318ef797bd8fdbc5bb714edbace042d4cb60111912218234e856f2cf300b3b47c91383b98e263ecf69c6c10193fef6c9581b", + }); +}