diff --git a/.changelog/unreleased/improvements/2845-limit-metadata-size.md b/.changelog/unreleased/improvements/2845-limit-metadata-size.md new file mode 100644 index 0000000000..ff436b85ab --- /dev/null +++ b/.changelog/unreleased/improvements/2845-limit-metadata-size.md @@ -0,0 +1,2 @@ +- Limit the character length of the validator metadata strings. + ([\#2845](https://github.com/anoma/namada/pull/2845)) \ No newline at end of file diff --git a/crates/apps/src/lib/config/genesis/transactions.rs b/crates/apps/src/lib/config/genesis/transactions.rs index a1b4213b38..446d172f1a 100644 --- a/crates/apps/src/lib/config/genesis/transactions.rs +++ b/crates/apps/src/lib/config/genesis/transactions.rs @@ -23,6 +23,7 @@ use namada::core::token; use namada::core::token::{DenominatedAmount, NATIVE_MAX_DECIMAL_PLACES}; use namada::ledger::pos::common::PublicKey; use namada::ledger::pos::types::ValidatorMetaData; +use namada::proof_of_stake::parameters::MAX_VALIDATOR_METADATA_LEN; use namada::tx::data::{pos, Fee, TxType}; use namada::tx::{ verify_standalone_sig, Code, Commitment, Data, Section, SignatureIndex, Tx, @@ -1356,6 +1357,54 @@ pub fn validate_validator_account( ); } + // Check that the validator metadata is not too large + let metadata = &signed_tx.data.metadata; + if metadata.email.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + panic!( + "The email metadata of the validator with address {} is too long, \ + must be within {MAX_VALIDATOR_METADATA_LEN} characters", + signed_tx.data.address + ); + } + if let Some(description) = metadata.description.as_ref() { + if description.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + panic!( + "The description metadata of the validator with address {} is \ + too long, must be within {MAX_VALIDATOR_METADATA_LEN} \ + characters", + signed_tx.data.address + ); + } + } + if let Some(website) = metadata.website.as_ref() { + if website.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + panic!( + "The website metadata of the validator with address {} is too \ + long, must be within {MAX_VALIDATOR_METADATA_LEN} characters", + signed_tx.data.address + ); + } + } + if let Some(discord_handle) = metadata.discord_handle.as_ref() { + if discord_handle.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + panic!( + "The discord handle metadata of the validator with address {} \ + is too long, must be within {MAX_VALIDATOR_METADATA_LEN} \ + characters", + signed_tx.data.address + ); + } + } + if let Some(avatar) = metadata.avatar.as_ref() { + if avatar.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + panic!( + "The avatar metadata of the validator with address {} is too \ + long, must be within {MAX_VALIDATOR_METADATA_LEN} characters", + signed_tx.data.address + ); + } + } + // Check signature let mut is_valid = { let maybe_threshold = { diff --git a/crates/proof_of_stake/src/parameters.rs b/crates/proof_of_stake/src/parameters.rs index 30d60b2b3e..f165c81fb1 100644 --- a/crates/proof_of_stake/src/parameters.rs +++ b/crates/proof_of_stake/src/parameters.rs @@ -132,6 +132,9 @@ pub enum ValidationError { UnbondingLenTooShort(u64, u64), } +/// The maximum string length of any validator metadata +pub const MAX_VALIDATOR_METADATA_LEN: u64 = 500; + /// The number of fundamental units per whole token of the native staking token pub const TOKENS_PER_NAM: u64 = 1_000_000; diff --git a/crates/sdk/src/error.rs b/crates/sdk/src/error.rs index 23cea67723..0aec671536 100644 --- a/crates/sdk/src/error.rs +++ b/crates/sdk/src/error.rs @@ -309,6 +309,9 @@ pub enum TxSubmitError { /// An empty string was provided as a new email #[error("An empty string cannot be provided as a new email")] InvalidEmail, + /// The metadata string is too long + #[error("The provided metadata string is too long")] + MetadataTooLong, /// The consensus key is not Ed25519 #[error("The consensus key must be an ed25519 key")] ConsensusKeyNotEd25519, diff --git a/crates/sdk/src/tx.rs b/crates/sdk/src/tx.rs index 15b69575d0..aab6b5889c 100644 --- a/crates/sdk/src/tx.rs +++ b/crates/sdk/src/tx.rs @@ -44,7 +44,9 @@ use namada_governance::storage::proposal::{ }; use namada_governance::storage::vote::ProposalVote; use namada_ibc::storage::channel_key; -use namada_proof_of_stake::parameters::PosParams; +use namada_proof_of_stake::parameters::{ + PosParams, MAX_VALIDATOR_METADATA_LEN, +}; use namada_proof_of_stake::types::{CommissionPair, ValidatorState}; use namada_token::storage_key::balance_key; use namada_token::DenominatedAmount; @@ -733,6 +735,68 @@ pub async fn build_validator_metadata_change( ); return Err(Error::from(TxSubmitError::InvalidEmail)); } + // Check that the email is within MAX_VALIDATOR_METADATA_LEN characters + if email.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + edisplay_line!( + context.io(), + "Email provided is too long, must be within \ + {MAX_VALIDATOR_METADATA_LEN} characters" + ); + if !tx_args.force { + return Err(Error::from(TxSubmitError::MetadataTooLong)); + } + } + } + + // Check that any new metadata provided is within MAX_VALIDATOR_METADATA_LEN + // characters + if let Some(description) = description.as_ref() { + if description.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + edisplay_line!( + context.io(), + "Description provided is too long, must be within \ + {MAX_VALIDATOR_METADATA_LEN} characters" + ); + if !tx_args.force { + return Err(Error::from(TxSubmitError::MetadataTooLong)); + } + } + } + if let Some(website) = website.as_ref() { + if website.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + edisplay_line!( + context.io(), + "Website provided is too long, must be within \ + {MAX_VALIDATOR_METADATA_LEN} characters" + ); + if !tx_args.force { + return Err(Error::from(TxSubmitError::MetadataTooLong)); + } + } + } + if let Some(discord_handle) = discord_handle.as_ref() { + if discord_handle.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + edisplay_line!( + context.io(), + "Discord handle provided is too long, must be within \ + {MAX_VALIDATOR_METADATA_LEN} characters" + ); + if !tx_args.force { + return Err(Error::from(TxSubmitError::MetadataTooLong)); + } + } + } + if let Some(avatar) = avatar.as_ref() { + if avatar.len() as u64 > MAX_VALIDATOR_METADATA_LEN { + edisplay_line!( + context.io(), + "Avatar provided is too long, must be within \ + {MAX_VALIDATOR_METADATA_LEN} characters" + ); + if !tx_args.force { + return Err(Error::from(TxSubmitError::MetadataTooLong)); + } + } } // If there's a new commission rate, it must be valid diff --git a/wasm/wasm_source/src/vp_user.rs b/wasm/wasm_source/src/vp_user.rs index 655d73ccd6..c5c6135dd7 100644 --- a/wasm/wasm_source/src/vp_user.rs +++ b/wasm/wasm_source/src/vp_user.rs @@ -15,6 +15,7 @@ use core::ops::Deref; use namada_vp_prelude::*; use once_cell::unsync::Lazy; +use proof_of_stake::parameters::MAX_VALIDATOR_METADATA_LEN; use proof_of_stake::storage::{read_pos_params, validator_state_handle}; use proof_of_stake::storage_key::{ is_below_capacity_validator_set_key, is_bond_epoched_meta_key, is_bond_key, @@ -188,11 +189,19 @@ fn validate_pos_changes( // Metadata changes must be signed by the validator whose // metadata is manipulated let is_valid_metadata_change = || { - let metadata = is_validator_metadata_key(key); - match metadata { - Some(address) => address == owner && **valid_sig, + let is_valid = match is_validator_metadata_key(key) { + Some(address) => { + let metadata = ctx.post().read::(key)?; + let valid_len = if let Some(metadata) = metadata { + (metadata.len() as u64) <= MAX_VALIDATOR_METADATA_LEN + } else { + true + }; + valid_len && address == owner && **valid_sig + } None => false, - } + }; + VpResult::Ok(is_valid) }; // Changes in validator state @@ -324,7 +333,7 @@ fn validate_pos_changes( || is_valid_reward_claim() || is_valid_redelegation() || is_valid_commission_rate_change() - || is_valid_metadata_change() + || is_valid_metadata_change()? || is_valid_become_validator() || **valid_sig) }