Skip to content

Commit

Permalink
[Eth]: Fix EIP712 message hashing when fixed byte array is 0x0 (#3522)
Browse files Browse the repository at this point in the history
  • Loading branch information
satoshiotomakan authored Oct 31, 2023
1 parent 6846db5 commit 72c67be
Show file tree
Hide file tree
Showing 4 changed files with 120 additions and 8 deletions.
23 changes: 20 additions & 3 deletions rust/tw_encoding/src/hex.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> = Result<T, FromHexError>;

pub trait ToHex {
fn to_hex(&self) -> String;
Expand All @@ -26,20 +29,34 @@ where
}

pub trait DecodeHex {
fn decode_hex(&self) -> Result<Vec<u8>, FromHexError>;
fn decode_hex(&self) -> FromHexResult<Data>;
}

impl<'a> DecodeHex for &'a str {
fn decode_hex(&self) -> Result<Vec<u8>, FromHexError> {
fn decode_hex(&self) -> FromHexResult<Data> {
decode(self)
}
}

pub fn decode(data: &str) -> Result<Vec<u8>, FromHexError> {
/// Decodes the given hexadecimal string.
pub fn decode(data: &str) -> FromHexResult<Data> {
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<Data> {
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<T: AsRef<[u8]>>(data: T, prefixed: bool) -> String {
let encoded = hex::encode(data.as_ref());
if prefixed {
Expand Down
9 changes: 4 additions & 5 deletions rust/tw_evm/src/message/eip712/eip712_message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -158,10 +158,9 @@ fn encode_fix_bytes(value: &Json, expected_len: usize) -> MessageSigningResult<D
let str = value
.as_str()
.ok_or(MessageSigningError::InvalidParameterValue)?;
let fix_bytes = str
.decode_hex()
.map_err(|_| MessageSigningError::InvalidParameterValue)?;
if fix_bytes.len() != expected_len {
let fix_bytes =
hex::decode_lenient(str).map_err(|_| MessageSigningError::InvalidParameterValue)?;
if fix_bytes.len() > expected_len {
return Err(MessageSigningError::TypeValueMismatch);
}
let checked_bytes =
Expand Down
83 changes: 83 additions & 0 deletions rust/tw_evm/tests/data/eip712_fixed_bytes.json
Original file line number Diff line number Diff line change
@@ -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"
}
}
13 changes: 13 additions & 0 deletions rust/tw_evm/tests/message_signer.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
});
}

0 comments on commit 72c67be

Please sign in to comment.