Hubble contest - 0xliumin's results

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/InsuranceFund.sol#L56

Vulnerability details

Impact

A user can get a much larger portion of the pool as it recovers from a debt seizure. The intent of the insurance pool seems to be that it could recover from a bad debt event.

Proof of Concept

1. Alice is the first LP to the insurance pool, and deposits 1e18 shares.
2. seizeBadDebt is called with 2e18. Now, there are pendingObligations = 1e18, and there is 0 vusd in the insurance fund.
3. Bob (the attacker) directly transfers 1e18 + 1 vUSD.
4. Bob calls deposit with 1e18 vUSD. All pending obligations will be settled, but there will only be 1 vUSD left in the pool before Bob's deposit. Bob receives shares = 1e18 * 1e18 / 1. As a result, Bob will get 1e36 shares, diluting Alice's share of the pool. Bob will be able to take a much larger share of all future profits from the insurance fund until more bad debt is seized. Bob only provided 2e18 + 1 liqudiity, but received an exponentially larger number of shares than Alice.

Mitigation

it depends on how you want this to work. You could keep track of the total amount ever contributed by users, and use that for calculations. Or just make staking 1 vUSD = 1 share if the pool total is below the total number of shares.

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/AMM.sol#L348

Vulnerability details

Impact

settleFunding calls will revert until all AMMs are ready to be updated.

Proof of Concept

1. AMM 1 has a nextFundingTime of now. AMM 2 has a nextFundingTime in 30 minutes. AMM 1 won't be able to be updated until after AMM 2's nextFundingTime elapses.

Mitigation

You shouldn't revert at the place mentioned in the links to affected code. Just return so that the other AMMs can still get updated.

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/ed1d885d5dbc2eae24e43c3ecbf291a0f5a52765/contracts/MarginAccount.sol#L377

Vulnerability details

Impact

Assets sent from MarginAccount to InsuranceFund will be locked forever

Proof of Concept

The insurance fund doesn't have a way to transfer non-vusd out of the contract.

Assets transferred to the InsuranceFund will be locked forever.

Mitigation

Have a way for governance to sweep tokens to swap them.