Lack of fund validation allows a user to bloat the message queue with empty stake messages

Summary

The lack of input validation in andromeda-validator-staking::execute_stake allows the message queue to be bloated with empty messages which would cause network congestion, downtimes and possible DOS of the application.

Vulnerability Detail

The Andromeda validator staking contract permits open staking in order to allow the contract_owner to stake from other contracts also owned by them. This is stated in the documentation.

fn execute_stake(ctx: ExecuteContext, validator: Option<Addr>) -> Result<Response, ContractError> {
    let ExecuteContext { deps, info, .. } = ctx;

    // Ensure only one type of coin is received
    ensure!(
        info.funds.len() == 1,
        ContractError::ExceedsMaxAllowedCoins {}
    );

    let default_validator = DEFAULT_VALIDATOR.load(deps.storage)?;

    // Use default validator if validator is not specified by stake msg
    let validator = validator.unwrap_or(default_validator);

    // Check if the validator is valid before staking
    is_validator(&deps, &validator)?;

    // Delegate funds to the validator

    //audit: No amount validation, queue can be bloated
@> let funds = &info.funds[0];

    let res = Response::new()
        .add_message(StakingMsg::Delegate {
            validator: validator.to_string(),
            amount: funds.clone(),
        })
        .add_attribute("action", "validator-stake")
        .add_attribute("from", info.sender)
        .add_attribute("to", validator.to_string())
        .add_attribute("amount", funds.amount);

    Ok(res)
}

However, since there is no whitelist functionality and also no check for the amount of funds to stake for !zero, any external users or group of malicious users can create numerous empty staking messages for the network validators. Since cosmos application runtime rely on computer resources to run effectively, by creating thousands of empty messages for example, this would cause network congestion, and potentially cause a DOS of the validator-staking application.

Impact

Validator staking contract can be DOSsed or slowed down severely by a bloated message queue.

Code Snippet

https://github.com/sherlock-audit/2024-05-andromeda-ado/blob/main/andromeda-core/contracts/finance/andromeda-validator-staking/src/contract.rs#L104-L136

Tool used

Manual Review

Recommendation

Include a check for if the amount to be staked is not 0. Also, consider including a minimum staking amount. This would discourage potential attacks due to financial limitations.

fn execute_stake(ctx: ExecuteContext, validator: Option<Addr>) -> Result<Response, ContractError> {
let ExecuteContext { deps, info, .. } = ctx;

// Ensure only one type of coin is received
ensure!(
    info.funds.len() == 1,
    ContractError::ExceedsMaxAllowedCoins {}
);

let default_validator = DEFAULT_VALIDATOR.load(deps.storage)?;

// Use default validator if validator is not specified by stake msg
let validator = validator.unwrap_or(default_validator);

// Check if the validator is valid before staking
is_validator(&deps, &validator)?;

// Delegate funds to the validator
let funds = &info.funds[0];
+  ensure!(
+     !funds.amount.is_zero(),
    ContractError::InvalidValidatorOperation {
        operation: "Stake".to_string(),
        validator: validator.to_string(),
    }
);
let res = Response::new()
    .add_message(StakingMsg::Delegate {
        validator: validator.to_string(),
        amount: funds.clone(),
    })
    .add_attribute("action", "validator-stake")
    .add_attribute("from", info.sender)
    .add_attribute("to", validator.to_string())
    .add_attribute("amount", funds.amount);

Ok(res)
}

Consider maintaining a hashmap of whitelisted addresses by the contract owner, since only the contract owner is expected to stake with other contracts owned by them

sherlock-audit / 2024-05-andromeda-ado-judging

forgebyola - Lack of fund validation allows a user to bloat the message queue with empty stake messages #78