From 3a64ad6a3a2d67c368775d70a70dc83722756b99 Mon Sep 17 00:00:00 2001 From: haiyizxx Date: Mon, 29 Jul 2024 15:51:43 -0400 Subject: [PATCH] feat(minor-voting-verifier): validate source address (#545) --- Cargo.lock | 3 + contracts/voting-verifier/Cargo.toml | 1 + contracts/voting-verifier/src/client.rs | 1 + contracts/voting-verifier/src/contract.rs | 85 ++++++++++++++++--- .../voting-verifier/src/contract/execute.rs | 69 +++++++++------ .../src/contract/migrations/v0_5_0.rs | 2 + contracts/voting-verifier/src/error.rs | 3 + contracts/voting-verifier/src/events.rs | 5 ++ contracts/voting-verifier/src/msg.rs | 2 + contracts/voting-verifier/src/state.rs | 2 + .../src/voting_verifier_contract.rs | 1 + .../tests/chain_freeze_unfreeze.rs | 2 +- integration-tests/tests/message_routing.rs | 2 +- packages/axelar-wasm-std/Cargo.toml | 1 + .../axelar-wasm-std/src/address_format.rs | 23 +++++ packages/axelar-wasm-std/src/lib.rs | 1 + 16 files changed, 166 insertions(+), 37 deletions(-) create mode 100644 packages/axelar-wasm-std/src/address_format.rs diff --git a/Cargo.lock b/Cargo.lock index 5c4bfe81e..75e735cdd 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -137,6 +137,7 @@ dependencies = [ "cfg-if", "const-hex", "derive_more", + "getrandom", "hex-literal", "itoa", "k256", @@ -793,6 +794,7 @@ checksum = "0c4b4d0bd25bd0b74681c0ad21497610ce1b7c91b1022cd21c80c6fbdd9476b0" name = "axelar-wasm-std" version = "0.1.0" dependencies = [ + "alloy-primitives", "bs58 0.5.1", "cosmwasm-schema", "cosmwasm-std", @@ -8852,6 +8854,7 @@ checksum = "2e4fe92cfc1bad19c19925d5eee4b30584dbbdee4ff10183b261acccbef74e2d" name = "voting-verifier" version = "0.5.0" dependencies = [ + "alloy-primitives", "axelar-wasm-std", "axelar-wasm-std-derive", "client", diff --git a/contracts/voting-verifier/Cargo.toml b/contracts/voting-verifier/Cargo.toml index d4f031a56..db7e0948b 100644 --- a/contracts/voting-verifier/Cargo.toml +++ b/contracts/voting-verifier/Cargo.toml @@ -52,6 +52,7 @@ service-registry = { workspace = true, features = ["library"] } thiserror = { workspace = true } [dev-dependencies] +alloy-primitives = { version = "0.7.7", features = ["getrandom"] } cw-multi-test = "0.15.1" integration-tests = { workspace = true } multisig = { workspace = true, features = ["test", "library"] } diff --git a/contracts/voting-verifier/src/client.rs b/contracts/voting-verifier/src/client.rs index a98d4de21..f0c107a01 100644 --- a/contracts/voting-verifier/src/client.rs +++ b/contracts/voting-verifier/src/client.rs @@ -222,6 +222,7 @@ mod test { source_chain: "source-chain".parse().unwrap(), rewards_address: "rewards".try_into().unwrap(), msg_id_format: axelar_wasm_std::msg_id::MessageIdFormat::HexTxHashAndEventIndex, + address_format: axelar_wasm_std::address_format::AddressFormat::Eip55, }; instantiate(deps, env, info.clone(), msg.clone()).unwrap(); diff --git a/contracts/voting-verifier/src/contract.rs b/contracts/voting-verifier/src/contract.rs index bfe20eb50..7ae5b495c 100644 --- a/contracts/voting-verifier/src/contract.rs +++ b/contracts/voting-verifier/src/contract.rs @@ -1,4 +1,4 @@ -use axelar_wasm_std::{permission_control, FnExt}; +use axelar_wasm_std::permission_control; #[cfg(not(feature = "library"))] use cosmwasm_std::entry_point; use cosmwasm_std::{ @@ -39,6 +39,7 @@ pub fn instantiate( source_chain: msg.source_chain, rewards_contract: deps.api.addr_validate(&msg.rewards_address)?, msg_id_format: msg.msg_id_format, + address_format: msg.address_format, }; CONFIG.save(deps.storage, &config)?; @@ -54,18 +55,25 @@ pub fn execute( msg: ExecuteMsg, ) -> Result { match msg.ensure_permissions(deps.storage, &info.sender)? { - ExecuteMsg::VerifyMessages(messages) => execute::verify_messages(deps, env, messages), - ExecuteMsg::Vote { poll_id, votes } => execute::vote(deps, env, info, poll_id, votes), - ExecuteMsg::EndPoll { poll_id } => execute::end_poll(deps, env, poll_id), + ExecuteMsg::VerifyMessages(messages) => Ok(execute::verify_messages(deps, env, messages)?), + ExecuteMsg::Vote { poll_id, votes } => Ok(execute::vote(deps, env, info, poll_id, votes)?), + ExecuteMsg::EndPoll { poll_id } => Ok(execute::end_poll(deps, env, poll_id)?), ExecuteMsg::VerifyVerifierSet { message_id, new_verifier_set, - } => execute::verify_verifier_set(deps, env, &message_id, new_verifier_set), + } => Ok(execute::verify_verifier_set( + deps, + env, + &message_id, + new_verifier_set, + )?), ExecuteMsg::UpdateVotingThreshold { new_voting_threshold, - } => execute::update_voting_threshold(deps, new_voting_threshold), - }? - .then(Ok) + } => Ok(execute::update_voting_threshold( + deps, + new_voting_threshold, + )?), + } } #[cfg_attr(not(feature = "library"), entry_point)] @@ -98,12 +106,15 @@ pub fn migrate( #[cfg(test)] mod test { + use axelar_wasm_std::address_format::AddressFormat; use axelar_wasm_std::msg_id::{ Base58SolanaTxSignatureAndEventIndex, Base58TxDigestAndEventIndex, HexTxHashAndEventIndex, MessageIdFormat, }; use axelar_wasm_std::voting::Vote; - use axelar_wasm_std::{nonempty, MajorityThreshold, Threshold, VerificationStatus}; + use axelar_wasm_std::{ + err_contains, nonempty, MajorityThreshold, Threshold, VerificationStatus, + }; use cosmwasm_std::testing::{ mock_dependencies, mock_env, mock_info, MockApi, MockQuerier, MockStorage, }; @@ -179,6 +190,7 @@ mod test { source_chain: source_chain(), rewards_address: REWARDS_ADDRESS.parse().unwrap(), msg_id_format: msg_id_format.clone(), + address_format: AddressFormat::Eip55, }, ) .unwrap(); @@ -237,7 +249,10 @@ mod test { .map(|i| Message { cc_id: CrossChainId::new(source_chain(), message_id("id", i, msg_id_format)) .unwrap(), - source_address: format!("source-address{i}").parse().unwrap(), + source_address: alloy_primitives::Address::random() + .to_string() + .try_into() + .unwrap(), destination_chain: format!("destination-chain{i}").parse().unwrap(), destination_address: format!("destination-address{i}").parse().unwrap(), payload_hash: [0; 32], @@ -269,7 +284,10 @@ mod test { Message { cc_id: CrossChainId::new(source_chain(), message_id("id", 1, &msg_id_format)) .unwrap(), - source_address: "source-address1".parse().unwrap(), + source_address: alloy_primitives::Address::random() + .to_string() + .parse() + .unwrap(), destination_chain: "destination-chain1".parse().unwrap(), destination_address: "destination-address1".parse().unwrap(), payload_hash: [0; 32], @@ -1350,4 +1368,49 @@ mod test { } }); } + + #[test] + fn should_fail_if_messages_have_invalid_source_address() { + let msg_id_format = MessageIdFormat::HexTxHashAndEventIndex; + let verifiers = verifiers(2); + let mut deps = setup(verifiers.clone(), &msg_id_format); + + let eip55_address = alloy_primitives::Address::random().to_string(); + + let cases = vec![ + eip55_address.to_lowercase(), + eip55_address.to_uppercase(), + // mix + eip55_address + .chars() + .enumerate() + .map(|(i, c)| { + if i % 2 == 0 { + c.to_uppercase().next().unwrap() + } else { + c.to_lowercase().next().unwrap() + } + }) + .collect::(), + ]; + + for case in cases { + let mut messages = messages(1, &MessageIdFormat::HexTxHashAndEventIndex); + messages[0].source_address = case.parse().unwrap(); + let msg = ExecuteMsg::VerifyMessages(messages); + let res = execute(deps.as_mut(), mock_env(), mock_info(SENDER, &[]), msg); + assert!(res.is_err_and(|err| err_contains!( + err.report, + ContractError, + ContractError::InvalidSourceAddress { .. } + ))); + } + + // should not fail if address is valid + let mut messages = messages(1, &MessageIdFormat::HexTxHashAndEventIndex); + messages[0].source_address = eip55_address.parse().unwrap(); + let msg = ExecuteMsg::VerifyMessages(messages); + let res = execute(deps.as_mut(), mock_env(), mock_info(SENDER, &[]), msg); + assert!(res.is_ok()); + } } diff --git a/contracts/voting-verifier/src/contract/execute.rs b/contracts/voting-verifier/src/contract/execute.rs index e2e2155bf..00dafdbbc 100644 --- a/contracts/voting-verifier/src/contract/execute.rs +++ b/contracts/voting-verifier/src/contract/execute.rs @@ -1,11 +1,14 @@ use std::collections::HashMap; +use axelar_wasm_std::address_format::{validate_address, AddressFormat}; +use axelar_wasm_std::utils::TryMapExt; use axelar_wasm_std::voting::{PollId, PollResults, Vote, WeightedPoll}; use axelar_wasm_std::{snapshot, MajorityThreshold, VerificationStatus}; use cosmwasm_std::{ to_json_binary, Deps, DepsMut, Env, Event, MessageInfo, OverflowError, OverflowOperation, QueryRequest, Response, Storage, WasmMsg, WasmQuery, }; +use error_stack::{report, Report, ResultExt}; use itertools::Itertools; use multisig::verifier_set::VerifierSet; use router_api::{ChainName, Message}; @@ -81,29 +84,22 @@ pub fn verify_messages( deps: DepsMut, env: Env, messages: Vec, -) -> Result { +) -> Result> { if messages.is_empty() { Err(ContractError::EmptyMessages)?; } - let source_chain = CONFIG.load(deps.storage)?.source_chain; + let config = CONFIG.load(deps.storage).map_err(ContractError::from)?; - if messages - .iter() - .any(|message| message.cc_id.source_chain != source_chain) - { - Err(ContractError::SourceChainMismatch(source_chain.clone()))?; - } - - let config = CONFIG.load(deps.storage)?; - - let messages = messages - .into_iter() - .map(|message| { - message_status(deps.as_ref(), &message, env.block.height) - .map(|status| (status, message)) - }) - .collect::, _>>()?; + let messages = messages.try_map(|message| { + validate_source_chain(message, &config.source_chain) + .and_then(|message| validate_source_address(message, &config.address_format)) + .and_then(|message| { + message_status(deps.as_ref(), &message, env.block.height) + .map(|status| (status, message)) + .map_err(Report::from) + }) + })?; let msgs_to_verify: Vec = messages .into_iter() @@ -121,18 +117,20 @@ pub fn verify_messages( return Ok(Response::new()); } - let snapshot = take_snapshot(deps.as_ref(), &source_chain)?; + let snapshot = take_snapshot(deps.as_ref(), &config.source_chain)?; let participants = snapshot.participants(); let expires_at = calculate_expiration(env.block.height, config.block_expiry.into())?; let id = create_messages_poll(deps.storage, expires_at, snapshot, msgs_to_verify.len())?; for (idx, message) in msgs_to_verify.iter().enumerate() { - poll_messages().save( - deps.storage, - &message.hash(), - &state::PollContent::::new(message.clone(), id, idx), - )?; + poll_messages() + .save( + deps.storage, + &message.hash(), + &state::PollContent::::new(message.clone(), id, idx), + ) + .map_err(ContractError::from)?; } let messages = msgs_to_verify @@ -366,3 +364,26 @@ fn calculate_expiration(block_height: u64, block_expiry: u64) -> Result Result> { + if message.cc_id.source_chain != *source_chain { + Err(report!(ContractError::SourceChainMismatch( + source_chain.clone() + ))) + } else { + Ok(message) + } +} + +fn validate_source_address( + message: Message, + address_format: &AddressFormat, +) -> Result> { + validate_address(&message.source_address, address_format) + .change_context(ContractError::InvalidSourceAddress)?; + + Ok(message) +} diff --git a/contracts/voting-verifier/src/contract/migrations/v0_5_0.rs b/contracts/voting-verifier/src/contract/migrations/v0_5_0.rs index 0a7a90651..06085bdcc 100644 --- a/contracts/voting-verifier/src/contract/migrations/v0_5_0.rs +++ b/contracts/voting-verifier/src/contract/migrations/v0_5_0.rs @@ -1,5 +1,6 @@ #![allow(deprecated)] +use axelar_wasm_std::address_format::AddressFormat; use axelar_wasm_std::error::ContractError; use axelar_wasm_std::msg_id::MessageIdFormat; use axelar_wasm_std::{nonempty, permission_control, MajorityThreshold}; @@ -43,6 +44,7 @@ fn migrate_config(storage: &mut dyn Storage, config: Config) -> StdResult<()> { msg_id_format: config.msg_id_format, source_gateway_address: config.source_gateway_address, voting_threshold: config.voting_threshold, + address_format: AddressFormat::Eip55, }; state::CONFIG.save(storage, &config) diff --git a/contracts/voting-verifier/src/error.rs b/contracts/voting-verifier/src/error.rs index 1aec7e088..f9995ca37 100644 --- a/contracts/voting-verifier/src/error.rs +++ b/contracts/voting-verifier/src/error.rs @@ -42,6 +42,9 @@ pub enum ContractError { #[error("poll results have different length")] PollResultsLengthUnequal, + + #[error("invalid source address")] + InvalidSourceAddress, } impl From for StdError { diff --git a/contracts/voting-verifier/src/events.rs b/contracts/voting-verifier/src/events.rs index a82fc5a0c..484171597 100644 --- a/contracts/voting-verifier/src/events.rs +++ b/contracts/voting-verifier/src/events.rs @@ -28,6 +28,7 @@ impl From for Vec { source_chain, rewards_contract, msg_id_format, + address_format, } = other; vec![ @@ -50,6 +51,10 @@ impl From for Vec { "msg_id_format", serde_json::to_string(&msg_id_format).expect("failed to serialize msg_id_format"), ), + ( + "address_format", + serde_json::to_string(&address_format).expect("failed to serialize address_format"), + ), ] .into_iter() .map(Attribute::from) diff --git a/contracts/voting-verifier/src/msg.rs b/contracts/voting-verifier/src/msg.rs index f0f75b078..dc6d79455 100644 --- a/contracts/voting-verifier/src/msg.rs +++ b/contracts/voting-verifier/src/msg.rs @@ -1,3 +1,4 @@ +use axelar_wasm_std::address_format::AddressFormat; use axelar_wasm_std::msg_id::MessageIdFormat; use axelar_wasm_std::voting::{PollId, PollStatus, Vote, WeightedPoll}; use axelar_wasm_std::{nonempty, MajorityThreshold, VerificationStatus}; @@ -30,6 +31,7 @@ pub struct InstantiateMsg { pub rewards_address: nonempty::String, /// Format that incoming messages should use for the id field of CrossChainId pub msg_id_format: MessageIdFormat, + pub address_format: AddressFormat, } #[cw_serde] diff --git a/contracts/voting-verifier/src/state.rs b/contracts/voting-verifier/src/state.rs index 1d687d5ee..471dc0ab0 100644 --- a/contracts/voting-verifier/src/state.rs +++ b/contracts/voting-verifier/src/state.rs @@ -1,3 +1,4 @@ +use axelar_wasm_std::address_format::AddressFormat; use axelar_wasm_std::hash::Hash; use axelar_wasm_std::msg_id::MessageIdFormat; use axelar_wasm_std::voting::{PollId, Vote, WeightedPoll}; @@ -21,6 +22,7 @@ pub struct Config { pub source_chain: ChainName, pub rewards_contract: Addr, pub msg_id_format: MessageIdFormat, + pub address_format: AddressFormat, } #[cw_serde] diff --git a/integration-tests/src/voting_verifier_contract.rs b/integration-tests/src/voting_verifier_contract.rs index b3342355d..43d4a2b60 100644 --- a/integration-tests/src/voting_verifier_contract.rs +++ b/integration-tests/src/voting_verifier_contract.rs @@ -51,6 +51,7 @@ impl VotingVerifierContract { .try_into() .unwrap(), msg_id_format: axelar_wasm_std::msg_id::MessageIdFormat::HexTxHashAndEventIndex, + address_format: axelar_wasm_std::address_format::AddressFormat::Eip55, }, &[], "voting_verifier", diff --git a/integration-tests/tests/chain_freeze_unfreeze.rs b/integration-tests/tests/chain_freeze_unfreeze.rs index 1886c5f9f..f1f46d0a8 100644 --- a/integration-tests/tests/chain_freeze_unfreeze.rs +++ b/integration-tests/tests/chain_freeze_unfreeze.rs @@ -21,7 +21,7 @@ fn chain_can_be_freezed_unfreezed() { "0x88d7956fd7b6fcec846548d83bd25727f2585b4be3add21438ae9fbb34625924-3", ) .unwrap(), - source_address: "0xBf12773B49()0e1Deb57039061AAcFA2A87DEaC9b9" + source_address: "0xBf12773B490e1Deb57039061AAcFA2A87DEaC9b9" .to_string() .try_into() .unwrap(), diff --git a/integration-tests/tests/message_routing.rs b/integration-tests/tests/message_routing.rs index 1c41efbc6..6ed73be9b 100644 --- a/integration-tests/tests/message_routing.rs +++ b/integration-tests/tests/message_routing.rs @@ -24,7 +24,7 @@ fn single_message_can_be_verified_and_routed_and_proven_and_rewards_are_distribu "0x88d7956fd7b6fcec846548d83bd25727f2585b4be3add21438ae9fbb34625924-3", ) .unwrap(), - source_address: "0xBf12773B49()0e1Deb57039061AAcFA2A87DEaC9b9" + source_address: "0xBf12773B490e1Deb57039061AAcFA2A87DEaC9b9" .to_string() .try_into() .unwrap(), diff --git a/packages/axelar-wasm-std/Cargo.toml b/packages/axelar-wasm-std/Cargo.toml index 61147911e..0adc3e5e6 100644 --- a/packages/axelar-wasm-std/Cargo.toml +++ b/packages/axelar-wasm-std/Cargo.toml @@ -29,6 +29,7 @@ optimize = """docker run --rm -v "$(pwd)":/code \ """ [dependencies] +alloy-primitives = { workspace = true } bs58 = "0.5.1" cosmwasm-schema = { workspace = true } cosmwasm-std = { workspace = true } diff --git a/packages/axelar-wasm-std/src/address_format.rs b/packages/axelar-wasm-std/src/address_format.rs new file mode 100644 index 000000000..b0de28b68 --- /dev/null +++ b/packages/axelar-wasm-std/src/address_format.rs @@ -0,0 +1,23 @@ +use alloy_primitives::Address; +use cosmwasm_schema::cw_serde; +use error_stack::{Report, ResultExt}; + +#[derive(thiserror::Error)] +#[cw_serde] +pub enum Error { + #[error("invalid address '{0}'")] + InvalidAddress(String), +} + +#[cw_serde] +pub enum AddressFormat { + Eip55, +} + +pub fn validate_address(address: &str, format: &AddressFormat) -> Result<(), Report> { + match format { + AddressFormat::Eip55 => Address::parse_checksummed(address, None) + .change_context(Error::InvalidAddress(address.to_string())) + .map(|_| ()), + } +} diff --git a/packages/axelar-wasm-std/src/lib.rs b/packages/axelar-wasm-std/src/lib.rs index 5ddcb9a68..3de4988a8 100644 --- a/packages/axelar-wasm-std/src/lib.rs +++ b/packages/axelar-wasm-std/src/lib.rs @@ -3,6 +3,7 @@ pub use crate::snapshot::{Participant, Snapshot}; pub use crate::threshold::{MajorityThreshold, Threshold}; pub use crate::verification::VerificationStatus; +pub mod address_format; pub mod counter; pub mod error; pub mod flagset;