sherlock-audit / 2023-04-hubble-exchange-judging

7 stars 6 forks source link

0x3e84fa45 - User can manipulate price oracles and drain the VUSD contract #189

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

0x3e84fa45

medium

User can manipulate price oracles and drain the VUSD contract

Summary

Hubble allows a user to withdraw their unrealized pnL in vUSD. An attacker can manipulate the oracle price and the market spot price to mint infinite vUSD.

Vulnerability Detail

The protocol accounts positive pnL towards the users margin. With enough unrealized PnL they can withdraw all of their assets and even borrow vUSD from the protocol. When the amount requested exceeds the balance of the contract, the Margin account mints more tokens.

Impact

An attacker can manipulate the chainlink and market spot price to increase their unrealized pnl. They then mint infinite vUSD and drain the vUSD contract.

Code Snippet

https://github.com/sherlock-audit/2023-04-hubble-exchange/blob/main/hubble-protocol/contracts/MarginAccount.sol#L651

Tool used

Manual Review

Recommendation

Don not account positive positive pnL towards the users margin.

function getNotionalPositionAndMarginVanilla(address trader, bool includeFundingPayments, Mode mode)
        public
        view
        returns(uint256 notionalPosition, int256 margin)
    {
        int256 unrealizedPnl;
        margin = marginAccount.getNormalizedMargin(trader);
        if (includeFundingPayments) {
            margin -= getTotalFunding(trader); // -ve fundingPayment means trader should receive funds
        }
        (notionalPosition, unrealizedPnl) = getTotalNotionalPositionAndUnrealizedPnl(trader, margin, mode);
-       margin += unrealizedPnl;
+       margin += min(unrealizedPnl, 0);
     }

The user should realize as profit before withdrawing it. Realizing profits at a manipulated price is not profitable.