Incorrect calculation of `total_shares` lead to inflated number of shares being minted

[hats-bug-reporter[bot]]

Github username: -- Twitter username: -- Submission hash (on-chain): 0xdf1bf7cb08c2a3b27bdb6ba658020ff034a5df2da159aabc9897bdc887ce637a Severity: high

Description: Description\

In the vault smart contract when user calls stake() function, the number of shares being minted to him is determined via get_shares_from_azero() that in its turn uses get_total_shares() that will inflate the number of shares to all the depositors coming after the first one.

Attack Scenario\

Consider the following scenario:

There are currently no any stakes so the shares for the first user is determined via

https://github.com/hats-finance/Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2/blob/main/src/vault/lib.rs#L809-813

if total_pooled_ == 0 {
                // This happens upon initial stake
                // Also known as 1:1 redemption ratio
                azero
            }

So suppose user1 deposited 100 shares => 100 shares are minted to him in this case. Now the total_pooled == 100 so the following formula for user2 is used (he deposited 100 azero next):

https://github.com/hats-finance/Kintsu-0x7d70f9442af3a9a0a734fa6a1b4857f25518e9d2/blob/main/src/vault/lib.rs#L814

 azero * self.get_total_shares() / total_pooled_

get_total_shares() is determined via this formula:

   pub fn get_total_shares(&self) -> Balance {
            self.data.total_shares_minted + self.get_current_virtual_shares()
        }

So it's 100 (total_shares_minted after the first user deposited + current virtual shares (for simplicity, let's say it's equal to 1 (equal to 1% fee) that was taken for the fee after user1 deposit as well). So the amount of shares for user2 will be determined the following way (say he deposits 100 as well):

100 * 101 (total minted shares + current virtual shares) / 100 = 101 shares

This way the shares are inflated and all the next depositors will get slightly more shares than the previous depositors as the value of current virtual shares is updated after each stake.

Recommendation

Change the fees mechanism how the fees are accounted. Preferably it's better not to use time difference but rather have an orientation on the change of total_shares_minted.

[rodiontr]

Sorry, recommendation is mistakenly copied as I was using the previous submission template to write this one. The recommendation is not to use the fees when calculating the mint amount. Consider implementing ERC4626 standard alike vault implementation

[0xmahdirostami]

Thank you for your submission. Duplicate of issue #25

check the comment on that issue