Hubble contest - danb's results

Lines of code

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

Vulnerability details

processWithdrawals can process limited amount in each call. an attacker can push to withdrawals enormous amount of withdrawals with amount = 0. in order to stop the dos attack and process the withdrawal, the governance needs to spend as much gas as the attacker. if the governance doesn't have enough money to pay for the gas, the withdrawals can't be processed.

Proof of Concept

Alice wants to attack vusd, she spend 1millions dollars for gas to push as many withdrawals of amount = 0 as she can. if the governance wants to process the deposits after alices empty deposits, they also need to spend at least 1 million dollars for gas in order to process alice's withdrawals first. but the governance doesn't have 1 million dollar so the funds will be locked.

Recommended Mitigation Steps

set a minimum amount of withdrawal. e.g. 1 dollar

    function withdraw(uint amount) external {
        require(amount >= 10 ** 6);
        burn(amount);
        withdrawals.push(Withdrawal(msg.sender, amount));
    }

Lines of code

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

Vulnerability details

in deposit, when the ratio totalSupply / balance is very high, the amount of the minted shares can round down to zero.

Proof of Concept

Alice is the first one to deposit to the insurance fund. she deposits 1 basic unit of vusd (10**-6 dollar), therefore minting one share. then she transferred 1 million vusd to the contract. then bob deposits 500,000 vusd. (500000 * 10**6 basic units) the amount of shares he gets is 500000 * 10**6 * 1 / (1000000* 10**6) = 0 therefore the number of shares didn't change but the balance increased by 500000 dollars. Alice can now withdraw her share and receive her funds back together with bob funds, as he doesn't have any shares.

Recommended Mitigation Steps

change to:

        if (_pool == 0) {
            shares = _amount * 10 ** 18;
        } else {

Lines of code

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

Vulnerability details

when an account is liquidated, there is no minimum amount of the swap, which makes it vulnerable for sandwich attacks.

Proof of Concept

Alice's long position can be liquidated, bob notices it and creates a short position, then liquidates her position, thus swapping the base asset to the quote asset, therefore reducing the base asset price, then he redeems his short position and profits because the price went down.

Tools Used

manual review

Recommended Mitigation Steps

set quoteAssetLimit in _reducePosition to prevent the attack

Lines of code

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

Vulnerability details

the governance can set the registry, therefore setting also the clearingHouse.

the clearingHouse has permissions to liquidate any trader.

also, they can set the clearingHouse in marginAccount and mint for themselves arbitrary amount of vusd, therefore they can steal all the funds.

Recomendation

don't allow the governance to change the registry.

already checked twice: https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/curve-v2/Swap.vy#L669 https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/curve-v2/Swap.vy#L671

the check can be removed, as it's an external view function: https://github.com/code-423n4/2022-02-hubble/blob/main/contracts/curve-v2/Swap.vy#L929