Hubble contest - minhquanym's results

1. VUSD.sol - withdrawals list can become too large so normal users will cost more gas to withdraw.

Impact

• Function withdraw() do not have check for amount > 0, an user can call it infinite time and make withdrawal list too large
• And because the function processWithdrawals() processes withdrawals in increasing order from the beginning of the withdrawal list. When a normal user wants to withdraw, they have to call the processWithdrawals() function multiple times to process all previous withdrawals.

Proof of Concept

• Call function withdraw() with input amount = 0 multiple times.

Recommended Mitigation Steps

• Add the check for amount > 0 in function withdraw()

Lines of code

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

Vulnerability details

Impact

• MarginAccount.sol - Line 377, token asset is transferred into insuranceFund but not found any function to transfer asset out of insuranceFund.

Proof of Concept

• Add any ERC20 token to Collateral list
• Call settleBadDebt()

Recommended Mitigation Steps

• Add function to transfer / approve any ERC20 token with role Admin