code-423n4 / 2024-03-dittoeth-findings

0 stars 0 forks source link

Flagged shorters can take advantage of forcedBidPriceBuffer to prevent liquidation by using a flash loan to place bids that fill all the asks/shorts below the forced bid price buffer. Then they can sell it back/close shorts in the same block. #208

Closed c4-bot-3 closed 4 months ago

c4-bot-3 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-03-dittoeth/blob/91faf46078bb6fe8ce9f55bcb717e5d2d302d22e/contracts/facets/PrimaryLiquidationFacet.sol#L99

Vulnerability details

Impact

The primary method of liquidation of shorts is in PrimaryLiquidationFacet.sol and it forces a shorter to place a bid on the market to buy the ERC stable asset to repay their ERC debt. But _checkLowestSell() will cause a call to liquidate() to fail if there are no asks or shorts available on the order book at less than 110% of the oracle price of the stable asset.

A flagged shorter can take advantage of this forced bid price buffer to prevent liquidation of their short at least temporarily by placing bids to fill all asks and shorts on the market at less than 110% of the oracle price. They would do this in the same block as the call to liquidate() for their short but pay higher gas so it is executed before liquidate(). Then they can sell what they bought and close their shorts (the onlyValidShortRecord modifier doesn't prevent

This will be more possible with thin liquidity but could be possible with deep liquidity using a flash loan. The goal of this could be to grief or frustrate the person trying to liquidate them or with the hope of preventing liquidation. It would waste the gas of the person trying to liquidate them because they don't get gas refunds if the call to liquidate() reverts. It could be done multiple times. There is a risk of loss of money on the part of the malicious flagged shorter because they aren't guaranteed to be able to sell back what they buy without a loss, and it costs gas. But they might be willing to do it to grief or frustrate their liquidator anyway.

Code

This is _checkLowestSell() which will revert (and cause liquidate() to revert) if there are no asks or shorts below the forced bid price buffer:

function _checklowestSell(MTypes.PrimaryLiquidation memory m) private view {
        uint16 lowestAskKey = s.asks[m.asset][C.HEAD].nextId;
        uint16 startingShortId = s.asset[m.asset].startingShortId;
        uint256 bufferPrice = m.oraclePrice.mul(m.forcedBidPriceBuffer);
        if (
            // Checks for no eligible asks
            (lowestAskKey == C.TAIL || s.asks[m.asset][lowestAskKey].price > bufferPrice)
            // Checks for no eligible shorts
            && (
                startingShortId == C.HEAD // means no short >= oracleprice
                    || s.shorts[m.asset][startingShortId].price > bufferPrice
            )
        ) {
            revert Errors.NoSells();
        }
    }

Proof of Concept

1) Malicious flagged shorter monitors mem pool for call to liquidate() for their short. Once they see it, they place enough bids (using high gas so it is executed before the call to liquidate() in the same block) to match with all the asks or shorts that are below the forced bid price buffer. They can use a flash loan to do this if it requires a lot of capital.

2) The call to liquidate() fails because there are no eligible shorts or asks.

3) Malicious flagged shorter can sell anything they bought back and close their shorts in the same block and repay flash loan if they used one. They can do this repeatedly if they want to grief or hoping to frustrate the liquidator and waste their gas (since they won't be refunded gas for uncompleted liquidations).

Tools Used

Manual review

Recommended Mitigation Steps

You could have a higher forced bid price buffer (which doesn't prevent this but would make it more expensive and less likely) or at least have a higher forced bid price buffer until there is deep liquidity on the order book. You could freeze the ability of an account with a flagged short from placing bids on the market while they have a flagged short (although that doesn't prevent them from using a different account to do this).

Assessed type

Other

c4-pre-sort commented 5 months ago

raymondfam marked the issue as insufficient quality report

c4-pre-sort commented 5 months ago

raymondfam marked the issue as primary issue

raymondfam commented 5 months ago

Readme: Issues related to front-running: can front-run someone's order, liquidation, the chainlink/uniswap oracle update.

c4-judge commented 4 months ago

hansfriese marked the issue as unsatisfactory: Out of scope