sherlock-audit / 2023-04-jojo-judging

7 stars 4 forks source link

monrel - Blacklisted users can not be liquidated if liquidateAmount > JUSDBorrowed #415

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

monrel

medium

Blacklisted users can not be liquidated if liquidateAmount > JUSDBorrowed

Summary

A USDC blacklisted address can not be liquidated if liquidateRemainUSDC >0 since transferring USDC to the blacklisted user will revert the transaction.

Vulnerability Detail

In JUSDBANK#liquidate() we see the following require statement:

        require(
            IERC20(primaryAsset).balanceOf(liquidated) -
                primaryLiquidatedAmount >=
                liquidateData.liquidatedRemainUSDC,
            JUSDErrors.LIQUIDATED_AMOUNT_NOT_ENOUGH
        )

Where primaryLiquidateAmount is the USDC balance before the liquidation. This require statements can only be passed if USDC is transferred to the blacklisted users in JUSDBank#_afterLiquidateOperation() or if primaryLiquidateAmount==0.

liquidateData.liquidatedRemainUSDC is >0 if liquidateAmount > JUSDBorrowed as seen in JUSDBank#_calcLiquidateAmount:

    function _calculateLiquidateAmount(
        ...
        ...
        if (liquidateAmount <= JUSDBorrowed) {
            liquidateData.actualCollateral = amount;
            liquidateData.insuranceFee = amount.decimalMul(priceOff).decimalMul(
                reserve.insuranceFeeRate
            );
            liquidateData.actualLiquidatedT0 = liquidateAmount.decimalDiv(
                tRate
            );
            liquidateData.actualLiquidated = liquidateAmount;
        } else {
            liquidateData.actualCollateral = JUSDBorrowed
                .decimalDiv(priceOff)
                .decimalDiv(JOJOConstant.ONE - reserve.insuranceFeeRate);
            liquidateData.insuranceFee = JUSDBorrowed
                .decimalMul(reserve.insuranceFeeRate) 
                .decimalDiv(JOJOConstant.ONE - reserve.insuranceFeeRate)
            liquidateData.actualLiquidatedT0 = liquidatedInfo.t0BorrowBalance;
            liquidateData.actualLiquidated = JUSDBorrowed;
        }

        liquidateData.liquidatedRemainUSDC = (amount -
            liquidateData.actualCollateral).decimalMul(price);
        ...
        ...

When this is the case liquidations will fail since transfers to blacklisted USDC users revert.

Impact

Liquidators are incentivized to liquidate as much collateral as possible since their profit is proportional to the amount liquidated. It is reasonable to assume that some will purposefully use amounts where liquidateAmount > JUSDBorrowed to guarantee that the maximum amount is liquidated to account for changes in price. This will always fail for blacklisted users.

The consequence is that blacklisted users are protected from these kinds of liquidations, this is unfair and could increase the risk of protocol debt.

Code Snippet

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/Impl/JUSDBank.sol#L199-L204

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/Impl/JUSDBank.sol#L410-L438

Tool used

Manual Review, vscode, foundry

Recommendation

Use internal accounting and allow liquidated users to pull out their USDC instead of transferring it during liquidation.

Duplicate of #206