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

1 stars 0 forks source link

g - Module registration lacks validation and can DOS ADO contracts #77

Closed sherlock-admin4 closed 3 months ago

sherlock-admin4 commented 3 months ago

g

Medium

Module registration lacks validation and can DOS ADO contracts

Summary

ADO contracts can register modules. However, these modules can have faulty addresses or non-unique addresses which can DOS some ADO contracts.

Vulnerability Detail

Registering a module in an ADO contract validates the uniqueness of a module's name but nothing else.

ref: https://github.com/sherlock-audit/2024-05-andromeda-ado/blob/main/andromeda-core/packages/std/src/ado_contract/modules/execute.rs#L22-L41

pub(crate) fn execute_register_module(
    // ... snip ...
    if should_validate {
        // checks that module has a unique name
        self.validate_modules(&self.load_modules(storage)?)?;
    }
    Ok(resp
        .add_attribute("action", "register_module")
        .add_attribute("module_idx", idx.to_string()))
}

ref: https://github.com/sherlock-audit/2024-05-andromeda-ado/blob/main/andromeda-core/packages/std/src/ado_contract/modules/mod.rs#L166-L172

fn validate_modules(&self, modules: &[Module]) -> Result<(), ContractError> {
    for module in modules {
        module.validate(modules)?;
    }

    Ok(())
}

ref: https://github.com/sherlock-audit/2024-05-andromeda-ado/blob/main/andromeda-core/packages/std/src/ado_base/modules.rs#L29-L50

pub fn validate(&self, modules: &[Module]) -> Result<(), ContractError> {
    ensure!(self.is_unique(modules), ContractError::ModuleNotUnique {});
    Ok(())
}

fn is_unique(&self, all_modules: &[Module]) -> bool {
    let mut total = 0;
    all_modules.iter().for_each(|m| {
        if self.name == m.name {
            total += 1;
        }
    });
    total == 1
}

A Module has an address (AndrAddr) and can be set to immutable. A module with a unique name is not necessarily unique among all registered modules. Multiple registered modules may point to the same address.

Also, there is no validation done on the address which can be a VFS path or a raw address.

Impact

Lockdrop ADO, CW20 Staking ADO, CW20 ADO, and Rate Limiting Withdrawals ADO all use module hooks in their execution handlers. A module hook queries all registered modules with a specified hook message. Any module with an invalid address will cause the module hook and the transaction to fail. This DOSes the contract unless the offending module is removed or updated.

However, immutable modules can not be removed or altered which leaves the DOS'd contract permanently unusable.

Code Snippet

Tool used

Manual Review

Recommendation

Consider validating the uniqueness of addresses among registered modules and validating the address with get_raw_address() when registering a new module.