hats-finance / Common--Stableswap-0xd4d9a2772202ce33b24901d3fc94e95a84b37430

Apache License 2.0
0 stars 0 forks source link

StablePoolContract : when creating the stable pool control, validation for some critical input is missing #16

Open hats-bug-reporter[bot] opened 4 months ago

hats-bug-reporter[bot] commented 4 months ago

Github username: -- Twitter username: -- Submission hash (on-chain): 0xf483e681910d54d806fa5a152d624e2f3b6ed90e7b02ff83e4ac7c8e274c6949 Severity: low

Description: Description\

When creatign the stable pool, the function does the follwong checks.

validate_amp_coef(amp_coef)?; calls a function to validate the amplification coefficient, returning an error if invalid.

unique_tokens.dedup(); removes duplicates.

ensure!(unique_tokens.len() == token_count, StablePoolError::IdenticalTokenId); checks if the number of unique tokens matches the original count, ensuring no duplicates.

ensure!(token_count == tokens_decimals.len() && token_count == token_rates.len() && (2..=MAX_COINS).contains(&token_count), StablePoolError::IncorrectTokenCount); ensures the number of tokens matches the length of tokens_decimals and token_rates, and that the count is within the allowed range (2 to MAX_COINS).

ensure!(tokens_decimals.iter().all(|&d| d <= TOKEN_TARGET_DECIMALS), StablePoolError::TooLargeTokenDecimal); ensures all token decimals are less than or equal to the target decimal value (TOKEN_TARGET_DECIMALS).

But, it misses to validate the following input when creating the pool.

ownerand fee reveiver. -- these two are missed in the validation.

Impact\

Missign these two would lead to unexected issue. either there will not be owner or fee reciver for the pool.

Attachments

  1. Revised Code File (Optional)

As done for other inputs, add validation for owner and fee receiver too.

JanKuczma commented 3 months ago

Thank you for your submission.

Could you give an example of how these two inputs could be validated? That is, validation against what?

aktech297 commented 3 months ago

Hi @JanKuczma ,

It would be like,

pub fn new_stable(
    tokens: Vec<AccountId>,
    tokens_decimals: Vec<u8>,
    init_amp_coef: u128,
    owner: Option<AccountId>,
    trade_fee: u32,
    protocol_fee: u32,
    fee_receiver: Option<AccountId>,
) -> Result<Self, StablePoolError> {

    // Check if owner is provided
    let owner = owner.ok_or(StablePoolError::OwnerNotProvided)?;

   //check for fee receiver also.
}
JanKuczma commented 3 months ago

The owner parameter is not of the Option<AccountId> type but of AccountId meaning it is guaranteed it is always present.

https://github.com/hats-finance/Common--Stableswap-0xd4d9a2772202ce33b24901d3fc94e95a84b37430/blob/e09a40c04d57311c9b9ce5e856ad89bc6571d036/amm/contracts/stable_pool/lib.rs#L168-L176