crimson-rat-reach - [HIGH] InsuranceFund#depositFor - Insurance Fund share mispricing can result in depositors getting 0 shares and attacker stealing all the funds #226
[HIGH] InsuranceFund#depositFor - Insurance Fund share mispricing can result in depositors getting 0 shares and attacker stealing all the funds
Summary
A malicious user with existing shares can front-run other deposit transactions so that other users get 0 shares, and the attacker can after that steal funds.
Vulnerability Detail
The depositFor() function in InsuranceFund.sol decides on the number of shares that a depositor should have in the following way:
The _pool value is determined by calling the _totalPoolValue() function, which iterates across all assets in the insurance fund pool and returns the sum of all assets.
Since the initial number of shares is determined by simply equating shares = amount, the attacker can follow these steps to steal user funds:
Make a deposit of a small amount, let us say 1 vUSD, by calling the depositFor() function. The attacker gets 1 share. Immediately call the unbond function as well.
Observe multiple subsequent deposit transactions, and let’s say the highest deposit transaction is of value x. However, the sum of values of this set of transactions must exceed x.
The attacker transfers a vUSD value of x+1 to the contract without calling the depositFor() function. This means that the total supply of shares minted is 1, but the pool value increases by x+1 vUSD.
The subsequent transactions, all less than the value of x+1vUSD go through, and all of them get 0 shares since shares = amount * (1 share) / (1+x+1 vUSD), and the amount is lesser than x+2.
After the unbonding period, the attacker withdraws a surplus of vUSD.
Impact
A malicious user can front-run the first insurance fund deposit and manipulate the insurance fund contract into issuing 0 shares for subsequent deposits and steal the insurance funds thereafter.
The team must add and burn a minimum liquidity value when deploying the contract and then only open it to the public. This must be set within the contract logic. Thus, such an attack becomes expensive.
The asset balances can be tracked within the contract state itself so that share mispricing is avoided.
crimson-rat-reach
high
[HIGH] InsuranceFund#depositFor - Insurance Fund share mispricing can result in depositors getting 0 shares and attacker stealing all the funds
Summary
A malicious user with existing shares can front-run other deposit transactions so that other users get 0 shares, and the attacker can after that steal funds.
Vulnerability Detail
The
depositFor()function inInsuranceFund.soldecides on the number of shares that a depositor should have in the following way:The
_poolvalue is determined by calling the_totalPoolValue()function, which iterates across all assets in the insurance fund pool and returns the sum of all assets.Since the initial number of shares is determined by simply equating shares = amount, the attacker can follow these steps to steal user funds:
Make a deposit of a small amount, let us say 1 vUSD, by calling the
depositFor()function. The attacker gets 1 share. Immediately call the unbond function as well.Observe multiple subsequent deposit transactions, and let’s say the highest deposit transaction is of value x. However, the sum of values of this set of transactions must exceed x.
The attacker transfers a vUSD value of
x+1to the contract without calling thedepositFor()function. This means that the total supply of shares minted is 1, but the pool value increases byx+1vUSD.The subsequent transactions, all less than the value of
x+1vUSD go through, and all of them get 0 shares sinceshares = amount * (1 share) / (1+x+1 vUSD), and the amount is lesser thanx+2.After the unbonding period, the attacker withdraws a surplus of vUSD.
Impact
A malicious user can front-run the first insurance fund deposit and manipulate the insurance fund contract into issuing 0 shares for subsequent deposits and steal the insurance funds thereafter.
Code Snippet
https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/InsuranceFund.sol#L89
https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/InsuranceFund.sol#L116
Tool used
Manual Review
Recommendation
There are two possible steps:
The team must add and burn a minimum liquidity value when deploying the contract and then only open it to the public. This must be set within the contract logic. Thus, such an attack becomes expensive.
The asset balances can be tracked within the contract state itself so that share mispricing is avoided.