Hubble contest - itsmeSTYJ's results

Lines of code

https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/InsuranceFund.sol#L71-L85 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/MarginAccount.sol#L352-L382

Vulnerability details

Impact

If pendingObligations are 0, it is possible to frontrun seizeBadDebt (which is called via MarginAccount.settleBadDebt). This allows a LP for the insurance fund to receive the benefits of providing liquidity to the insurance fund without incurring any “loss” whenever bad debts are created. These “loss” are instead pushed onto the other LPs i.e. they are incuring more “loss” than they should.

Proof of Concept

When someone tries to call MarginAccount.settleBadDebt and InsuranceFund.pendingObligations are 0, you can frontrun this user and call InsuranceFund.withdraw to withdraw your LP before their transaction gets mined. After their transaction is mined, you can call InsuranceFund.deposit to put your LP back into the insurance fund to continue reaping the rewards.

Tools Used

Recommended Mitigation Steps

• Implement a withdraw request process so that whenever you want to withdraw, you need to first make a withdraw request and wait X amt of blocks.

QA Report

Low Findings

L01: Add modifiers to ensure that dependencies are synced

Description

The syncDeps function exists in the AMM, InsuranceFund and MarginAccount contracts as a helper function to sync the shared contract addresses between the Registry contract. It does not however ensure that all contracts are actually in synced i.e. if the Registry contract changes, there are no checks to ensure that the addresses are still in sync.

L02: Adhere to the Check Effect Interactions pattern best practice

Line References

https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/VUSD.sol#L62-L64

Description

Update internal state before transferring tokens out.

Non-Critical Findings

NC01: Avoid rebase tokens

Line References

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

Description

Be mindful when adding collateral as some token implementations e.g. rebase tokens are not handled by the smart contract.

NC02: Incorrect comment for _getLiquidationInfo

Line References

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

Description

_getLiquidationInfo doesn’t actually revert. It is the parent liquidation function that reverts if it receives a LiquidationStatus that is not IS_LIQUIDATABLE.

NC03: Add a rescue token / ETH function

Description

In the event ETH / tokens are mistakenly sent to the contracts, it is possible to withdraw them through a rescueToken or rescueETH function. This function can only be called by governance.