sherlock-audit / 2023-04-jojo-judging

7 stars 4 forks source link

monrel - Liquidator can steal collateral by issuing a loan to liquidated user #479

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

monrel

medium

Liquidator can steal collateral by issuing a loan to liquidated user

Summary

A liquidator can steal parts or all of the remaining collateral in a liquidation if the liquidated user has a bid to take a loan on a P2P market.

Vulnerability Detail

When a user is liquidated the remaining collateral is sent to the liquidated user.

This is enforced in JUSDBank#liquidate():

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

liquidateData.liquidatedRemainUSDC is the value of the remaining collateral after their debt has been completely repaid. This can be seen in JUSDBank#_calculateLiquidateAmount():

function _calculateLiquidateAmount(
    ...
    ...
    ...
uint256 liquidateAmount = amount.decimalMul(priceOff).decimalMul(
            JOJOConstant.ONE - reserve.insuranceFeeRate
        );
        uint256 JUSDBorrowed = liquidatedInfo.t0BorrowBalance.decimalMul(tRate);

        if (liquidateAmount <= JUSDBorrowed) {
            ...
            ...
            ...
        } else {
insuranceFeeRate)
            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 - //@audit accountfor fee?
            liquidateData.actualCollateral).decimalMul(price);
    }

Assuming the liquidated user is seeking a loan on a P2P lending market with a bid that can be taken by any lender.

The liquidator can offer this loan to the liquidated user in a contract that they have deployed and is called in JUSDBank#_afterLiquidateOperation():

        IFlashLoanReceive(flashloanAddress).JOJOFlashLoan(
            collateral,
            flashloanAmount,
            liquidated,
            param
        );

In this contract, a function with the same signature as JOJOFlashLoan will do the following:

  1. Take the bid of the liquidated user, the liquidated user will receive USDC.
  2. If loan > iquidateData.liquidatedRemainUSDC all of the remaining USDC will be sent to the attacker instead of the liquidated user otherwise loan amount will be stolen.

The liquidated user will now have lost an amount of the collateral. They have entered into a loan that they will have to repay or default on.

Impact

Liquidators can steal from users with unsafe positions if they have a bid to take a loan in USDC on P2P lending platform.

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#L379-L438

https://github.com/sherlock-audit/2023-04-jojo/blob/main/JUSDV1/src/Impl/flashloanImpl/FlashLoanLiquidate.sol#L46-L95

Tool used

Manual Review, vscode, foundry

Recommendation

Remove the dependency on the USDC balance of the liquidated user. Add new functionality to allow liquidators to deposit USDC into the JUSDBank (or another separate contract) and allow liquidators to pull those funds out. The change in the amount deposited this way should govern if enough has been sent to the liquidated user.