diff --git a/contracts/wormhole/src/contract.rs b/contracts/wormhole/src/contract.rs index 7178c6287..01924d94c 100644 --- a/contracts/wormhole/src/contract.rs +++ b/contracts/wormhole/src/contract.rs @@ -123,7 +123,8 @@ fn handle_governance_payload(deps: DepsMut, env: Env, data: &[u8]) -> StdResult< let gov_packet = GovernancePacket::deserialize(data)?; let state = CONFIG.load(deps.storage)?; - let module = String::from_utf8(gov_packet.module).unwrap(); + let module = String::from_utf8(gov_packet.module) + .map_err(|_| StdError::msg("invalid module encoding"))?; let module: String = module.chars().filter(|c| c != &'\0').collect(); if module != "Core" { @@ -235,6 +236,10 @@ fn vaa_update_guardian_set(deps: DepsMut, env: Env, data: &[u8]) -> StdResult StdResult { + if data.len() < Self::HEADER_LEN { + return ContractError::InvalidVAA.std_err(); + } + let version = data.get_u8(0); // Load 4 bytes starting from index 1 @@ -207,9 +211,8 @@ pub struct GuardianSetInfo { impl GuardianSetInfo { pub fn quorum(&self) -> usize { - // allow quorum of 0 for testing purposes... if self.addresses.is_empty() { - return 0; + return 1; } ((self.addresses.len() * 10 / 3) * 2) / 10 + 1 } @@ -250,6 +253,7 @@ pub fn vaa_archive_check(storage: &dyn Storage, hash: &[u8]) -> bool { VAA_ARCHIVE.may_load(storage, hash).unwrap_or(Some(false)).unwrap_or(false) } +#[derive(Debug)] pub struct GovernancePacket { pub module: Vec, pub action: u8, @@ -258,7 +262,13 @@ pub struct GovernancePacket { } impl GovernancePacket { + const MIN_LEN: usize = 35; // 32-byte module + 1-byte action + 2-byte chain + pub fn deserialize(data: &[u8]) -> StdResult { + if data.len() < Self::MIN_LEN { + return ContractError::InvalidVAA.std_err(); + } + let module = data.get_bytes32(0).to_vec(); let action = data.get_u8(32); let chain = data.get_u16(33); diff --git a/contracts/wormhole/src/testing.rs b/contracts/wormhole/src/testing.rs index d8467fbf7..30bae8b23 100644 --- a/contracts/wormhole/src/testing.rs +++ b/contracts/wormhole/src/testing.rs @@ -1,11 +1,12 @@ use cosmwasm_std::testing::{message_info, mock_dependencies, mock_env}; use cosmwasm_std::{from_json, Binary, Coin, Uint256}; -use crate::contract::{instantiate, query}; +use crate::contract::{instantiate, execute, query}; use crate::msg::{ - GetAddressHexResponse, GetStateResponse, GuardianSetInfoResponse, InstantiateMsg, QueryMsg, + ExecuteMsg, GetAddressHexResponse, GetStateResponse, GuardianSetInfoResponse, InstantiateMsg, + QueryMsg, }; -use crate::state::{GuardianAddress, GuardianSetInfo}; +use crate::state::{GovernancePacket, GuardianAddress, GuardianSetInfo, ParsedVAA}; const GOV_ADDRESS: &str = "AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAE"; const GUARDIAN_ADDR: &str = "54dbb737eac5007103e729e9ab7ce64a6850a310"; @@ -179,12 +180,12 @@ fn test_quorum_calculation() { }; assert_eq!(three.quorum(), 3); - // Empty set: quorum = 0 + // Empty set: quorum = 1 (never allow zero quorum) let empty = GuardianSetInfo { addresses: vec![], expiration_time: 0, }; - assert_eq!(empty.quorum(), 0); + assert_eq!(empty.quorum(), 1); // 19 guardians (mainnet-like): quorum = 13 let nineteen = GuardianSetInfo { @@ -195,3 +196,189 @@ fn test_quorum_calculation() { }; assert_eq!(nineteen.quorum(), 13); } + +// --------------------------------------------------------------------------- +// H-1: ParsedVAA::deserialize rejects short input instead of panicking +// --------------------------------------------------------------------------- + +#[test] +fn test_deserialize_vaa_empty_input() { + let result = ParsedVAA::deserialize(&[]); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("InvalidVAA")); +} + +#[test] +fn test_deserialize_vaa_short_input() { + // 3 bytes — shorter than HEADER_LEN (6) + let result = ParsedVAA::deserialize(&[0x01, 0x02, 0x03]); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("InvalidVAA")); +} + +#[test] +fn test_deserialize_vaa_exactly_header_len() { + // 6 bytes = HEADER_LEN, but body_offset will exceed data.len() with 0 signers + // version=1, guardian_set_index=0, len_signers=0 → body_offset=6, data.len()=6 + // body_offset >= data.len() → InvalidVAA (existing check) + let data = [0x01, 0x00, 0x00, 0x00, 0x00, 0x00]; + let result = ParsedVAA::deserialize(&data); + assert!(result.is_err()); +} + +#[test] +fn test_short_vaa_via_execute() { + let mut deps = mock_dependencies(); + let env = mock_env(); + let info = message_info(&deps.api.addr_make("creator"), &[]); + let msg = mock_instantiate_msg(0); + instantiate(deps.as_mut(), env.clone(), info.clone(), msg).unwrap(); + + let short_vaa = Binary::from(vec![0x01, 0x02, 0x03]); + let res = execute( + deps.as_mut(), + env, + info, + ExecuteMsg::SubmitVAA { vaa: short_vaa }, + ); + assert!(res.is_err()); + assert!(res.unwrap_err().to_string().contains("InvalidVAA")); +} + +#[test] +fn test_short_vaa_via_query() { + let mut deps = mock_dependencies(); + let env = mock_env(); + let info = message_info(&deps.api.addr_make("creator"), &[]); + let msg = mock_instantiate_msg(0); + instantiate(deps.as_mut(), env.clone(), info, msg).unwrap(); + + let short_vaa = Binary::from(vec![0x01, 0x02, 0x03]); + let res = query( + deps.as_ref(), + env, + QueryMsg::VerifyVAA { + vaa: short_vaa, + block_time: 0, + }, + ); + assert!(res.is_err()); + assert!(res.unwrap_err().to_string().contains("InvalidVAA")); +} + +// --------------------------------------------------------------------------- +// M-3: Empty guardian set now requires quorum of 1 (blocks unsigned VAAs) +// --------------------------------------------------------------------------- + +#[test] +fn test_empty_guardian_set_rejects_unsigned_vaa() { + let mut deps = mock_dependencies(); + let env = mock_env(); + let info = message_info(&deps.api.addr_make("creator"), &[]); + + // Instantiate with an empty guardian set + let msg = InstantiateMsg { + gov_chain: 1, + gov_address: Binary::from(hex::decode(GOV_ADDRESS).unwrap()), + initial_guardian_set: GuardianSetInfo { + addresses: vec![], + expiration_time: 0, + }, + guardian_set_index: 0, + guardian_set_expirity: 86400, + chain_id: 32, + fee_denom: "uakt".to_string(), + }; + instantiate(deps.as_mut(), env.clone(), info.clone(), msg).unwrap(); + + // Build a minimal valid-structure VAA with 0 signatures: + // version(1) + guardian_set_index(4) + len_signers(1) = 6 bytes header + // body needs at least VAA_PAYLOAD_POS(51) bytes + let mut vaa = Vec::new(); + vaa.push(0x01); // version + vaa.extend_from_slice(&0u32.to_be_bytes()); // guardian_set_index = 0 + vaa.push(0x00); // len_signers = 0 + // body: 51 bytes minimum (timestamp + nonce + emitter_chain + emitter_address + sequence + consistency_level) + vaa.extend_from_slice(&[0u8; 51]); + + let res = execute( + deps.as_mut(), + env, + info, + ExecuteMsg::SubmitVAA { vaa: Binary::from(vaa) }, + ); + assert!(res.is_err()); + assert!(res.unwrap_err().to_string().contains("NoQuorum")); +} + +// --------------------------------------------------------------------------- +// M-4: GovernancePacket returns error on invalid UTF-8 instead of panicking +// --------------------------------------------------------------------------- + +#[test] +fn test_governance_packet_invalid_utf8_module() { + // 32-byte module with invalid UTF-8, followed by action + chain + let mut data = vec![0xFF; 32]; // invalid UTF-8 bytes + data.push(0x01); // action + data.extend_from_slice(&0u16.to_be_bytes()); // chain + + let packet = GovernancePacket::deserialize(&data); + // Deserialization succeeds — it just stores raw bytes + assert!(packet.is_ok()); + + // The panic was in contract.rs where String::from_utf8().unwrap() was called. + // Verify that from_utf8 on these bytes would fail: + let packet = packet.unwrap(); + assert!(String::from_utf8(packet.module).is_err()); +} + +// --------------------------------------------------------------------------- +// M-6: GovernancePacket::deserialize rejects short input instead of panicking +// --------------------------------------------------------------------------- + +#[test] +fn test_governance_packet_empty_input() { + let result = GovernancePacket::deserialize(&[]); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("InvalidVAA")); +} + +#[test] +fn test_governance_packet_short_input() { + // 10 bytes — shorter than MIN_LEN (35) + let result = GovernancePacket::deserialize(&[0u8; 10]); + assert!(result.is_err()); + assert!(result.unwrap_err().to_string().contains("InvalidVAA")); +} + +#[test] +fn test_governance_packet_exactly_min_len() { + // Exactly 35 bytes — should succeed with an empty payload + let mut data = vec![0u8; 32]; // module (all zeros = null-padded) + data.push(0x02); // action + data.extend_from_slice(&0u16.to_be_bytes()); // chain = 0 + + assert_eq!(data.len(), 35); + let result = GovernancePacket::deserialize(&data); + assert!(result.is_ok()); + let packet = result.unwrap(); + assert_eq!(packet.action, 2); + assert_eq!(packet.chain, 0); + assert!(packet.payload.is_empty()); +} + +#[test] +fn test_governance_packet_with_payload() { + // 35 + 4 bytes of payload + let mut data = vec![0u8; 32]; // module + data.push(0x01); // action + data.extend_from_slice(&32u16.to_be_bytes()); // chain = 32 + data.extend_from_slice(&[0xDE, 0xAD, 0xBE, 0xEF]); // payload + + let result = GovernancePacket::deserialize(&data); + assert!(result.is_ok()); + let packet = result.unwrap(); + assert_eq!(packet.action, 1); + assert_eq!(packet.chain, 32); + assert_eq!(packet.payload, vec![0xDE, 0xAD, 0xBE, 0xEF]); +}