Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

InsuranceFund depositors can be priced out & deposits can be stolen #42

Open
code423n4 opened this issue Feb 21, 2022 · 1 comment
Open
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working

Comments

@code423n4
Copy link
Contributor

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/8c157f519bc32e552f8cc832ecc75dc381faa91e/contracts/InsuranceFund.sol#L44-L54

Vulnerability details

Impact

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.

@code423n4 code423n4 added 3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working labels Feb 21, 2022
code423n4 added a commit that referenced this issue Feb 21, 2022
@atvanguard
Copy link
Collaborator

Duplicate of #116

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3 (High Risk) Assets can be stolen/lost/compromised directly bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants