hats-finance / Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2

Smart contracts for Kintsu
Other
0 stars 0 forks source link

The Vault data total_pooled can be manipulated during yield compounding #55

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

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

Github username: @veselinas Twitter username: -- Submission hash (on-chain): 0x3365b23a6c0385ecf8cfbfc1b528dec1e8312fb59230767b2e2929f86c62a624 Severity: low

Description: Description

In the src/vault/data.rs, in particular in the method delegate_compound, the total_pooled AZERO balance on the Vault data will be incorrect if an internal error is encountered. During the for loop, we can successfully update the compounded yield for a portion of the nominator agents before encountering an error. Due to the occurrence of the error we won't reach the line which updates the total_pooled. As a result the total_pooled balance will be incorrect. This has a direct impact on the redemption ratio.

Attack Scenario

Assuming we have multiple nominator agents and we enter the for loop presented in the proof of concept. As a result of a hacker attack call_compound might fail midway in the for loop (i.e. after a few successful iterations have taken place resulting in the balance for multiple agents to be updated). As a result, the function will exit early and the total_pooled balance won't be updated i.e. it won't reflect the actual AZERO tokens which were staked but not unbonded.

Attachments

  1. Proof of Concept (PoC) File

    for (i, a) in agents.into_iter().enumerate() {
        match call_compound(a.address, incentive_percentage_) {
            Ok((compound_amount, incentive_amount)) => {
                debug_println!("Compounded {} to agent #{}", compound_amount, i);
                total_compounded += compound_amount;
                total_incentive += incentive_amount;
            },
            Err(e) => return Err(VaultError::InternalError(e)),
        }
    }
    
    if total_compounded == 0 {
        return Err(VaultError::ZeroCompounding);
    }
    
    self.total_pooled += total_compounded;
  2. Revised Code File (Optional)

Incrementing the total_pooled balance iteratively during the for loop instead of once after the for loop end will prevent this issue.

for (i, a) in agents.into_iter().enumerate() {
        match call_compound(a.address, incentive_percentage_) {
            Ok((compound_amount, incentive_amount)) => {
                debug_println!("Compounded {} to agent #{}", compound_amount, i);
                total_compounded += compound_amount;
              self.total_pooled += compound_amount;
                total_incentive += incentive_amount;
            },
            Err(e) => return Err(VaultError::InternalError(e)),
        }
    }

    if total_compounded == 0 {
        return Err(VaultError::ZeroCompounding);
    }
bgibers commented 4 months ago

if a transaction reverts, all state changes revert