The InsuranceFund.deposit function mints initial shares equal to the deposited amount.
The deposit / withdraw functions also use the VUSD contract balance for the shares computation. (balance() = vusd.balanceOf(address(this)))
It's possible to increase the share price to very high amounts and price out smaller depositors.
POC
deposit(_amount = 1): Deposit the smallest unit of VUSD as the first depositor. Mint 1 share and set the total supply and VUSD balance to 1.
Perform a direct transfer of 1000.0 VUSD to the InsuranceFund. The balance() is now 1000e6 + 1
Doing any deposits of less than 1000.0 VUSD will mint zero shares: shares = _amount * _totalSupply / _pool = 1000e6 * 1 / (1000e6 + 1) = 0.
The attacker can call withdraw(1) to burn their single share and receive the entire pool balance, making a profit. (balance() * _shares / totalSupply() = balance())
I give this a high severity as the same concept can be used to always steal the initial insurance fund deposit by frontrunning it and doing the above-mentioned steps, just sending the frontrunned deposit amount to the contract instead of the fixed 1000.0.
They can then even repeat the steps to always frontrun and steal any deposits.
Recommended Mitigation Steps
The way UniswapV2 prevents this is by requiring a minimum deposit amount and sending 1000 initial shares to the zero address to make this attack more expensive.
The same mitigation can be done here.
atvanguard
commented
2 years ago
Lines of code
https://github.com/code-423n4/2022-02-hubble/blob/8c157f519bc32e552f8cc832ecc75dc381faa91e/contracts/InsuranceFund.sol#L44-L54
Vulnerability details
Impact
The
InsuranceFund.depositfunction mints initialsharesequal to the deposited amount. The deposit / withdraw functions also use the VUSD contract balance for the shares computation. (balance() = vusd.balanceOf(address(this)))It's possible to increase the share price to very high amounts and price out smaller depositors.
POC
deposit(_amount = 1): Deposit the smallest unit of VUSD as the first depositor. Mint 1 share and set the total supply and VUSD balance to1.1000.0VUSD to theInsuranceFund. Thebalance()is now1000e6 + 11000.0VUSD will mint zero shares:shares = _amount * _totalSupply / _pool = 1000e6 * 1 / (1000e6 + 1) = 0.withdraw(1)to burn their single share and receive the entire pool balance, making a profit. (balance() * _shares / totalSupply() = balance())I give this a high severity as the same concept can be used to always steal the initial insurance fund deposit by frontrunning it and doing the above-mentioned steps, just sending the frontrunned deposit amount to the contract instead of the fixed
1000.0. They can then even repeat the steps to always frontrun and steal any deposits.Recommended Mitigation Steps
The way UniswapV2 prevents this is by requiring a minimum deposit amount and sending
1000initial shares to the zero address to make this attack more expensive. The same mitigation can be done here.