sherlock-audit / 2023-04-jojo-judging

7 stars 4 forks source link

Aymen0909 - No slippage protection in `FlashLoanLiquidate` #466

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

Aymen0909

medium

No slippage protection in FlashLoanLiquidate

Summary

The JOJOFlashLoan function inside FlashLoanLiquidate does no implement slippage protection when swapping to USDC, so user can potentially lose funds when calling this function in case of a high slippage.

Vulnerability Detail

The issue occurs in the JOJOFlashLoan function below :

function JOJOFlashLoan(
    address asset,
    uint256 amount,
    address to,
    bytes calldata param
) external {
    //swapContract swap
    (LiquidateData memory liquidateData, bytes memory originParam) = abi
        .decode(param, (LiquidateData, bytes));
    (
        address approveTarget,
        address swapTarget,
        address liquidator,
        bytes memory data
    ) = abi.decode(originParam, (address, address, address, bytes));
    IERC20(asset).approve(approveTarget, amount);
    (bool success, ) = swapTarget.call(data);
    if (success == false) {
        assembly {
            let ptr := mload(0x40)
            let size := returndatasize()
            returndatacopy(ptr, 0, size)
            revert(ptr, size)
        }
    }

    // @audit No check on the USDC amount received after the swap
    // Put the user at risk in the case of high slippage
    uint256 USDCAmount = IERC20(USDC).balanceOf(address(this));

    IERC20(USDC).approve(jusdExchange, liquidateData.actualLiquidated);
    IJUSDExchange(jusdExchange).buyJUSD(
        liquidateData.actualLiquidated,
        address(this)
    );
    IERC20(JUSD).approve(jusdBank, liquidateData.actualLiquidated);
    IJUSDBank(jusdBank).repay(liquidateData.actualLiquidated, to);

    // 2. insurance
    IERC20(USDC).safeTransfer(insurance, liquidateData.insuranceFee);
    // 3. liquidate usdc
    if (liquidateData.liquidatedRemainUSDC != 0) {
        IERC20(USDC).safeTransfer(to, liquidateData.liquidatedRemainUSDC);
    }
    // 4. transfer to liquidator
    IERC20(USDC).safeTransfer(
        liquidator,
        USDCAmount -
            liquidateData.insuranceFee -
            liquidateData.actualLiquidated -
            liquidateData.liquidatedRemainUSDC
    );
}

As you can see after swapping from ERC20 asset to USDC the function uses directly the USDC amount received by calling IERC20(USDC).balanceOf(address(this)) and it does not check if the received amount is the one expected by the user, so in case of high slippage the user will lose funds.

It's worth noting that the other contracts that are also responsible of flashloans operations : FlashLoanRepay and GeneralRepay do contain slippage proctection when swapping to USDC.

Impact

See summary

Code Snippet

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

Tool used

Manual Review

Recommendation

I recommend to add a slippage protection in the JOJOFlashLoan function, this can be done as in the FlashLoanRepay and GeneralRepay contracts by adding a minReceive variable in the call and checking if the returned USDC amount is above it or not.

Duplicate of #373