c4-bot-3 commented 4 months ago

Lines of code

https://github.com/code-423n4/2024-03-revert-lend/blob/main/src/utils/Swapper.sol#L73-L118

Vulnerability details

Impact

The Swapper::_routerSwap function does not check whether the amount of tokens received from the swap is equal to the expected amount of tokens. This can result in a loss of funds for the user if the swap returns less tokens than expected.

Proof of Concept

    function _routerSwap(RouterSwapParams memory params)
        internal
        returns (uint256 amountInDelta, uint256 amountOutDelta)
    {
        if (params.amountIn != 0 && params.swapData.length != 0 && address(params.tokenOut) != address(0)) {
            uint256 balanceInBefore = params.tokenIn.balanceOf(address(this));
            uint256 balanceOutBefore = params.tokenOut.balanceOf(address(this));

            // get router specific swap data
            (address router, bytes memory routerData) = abi.decode(params.swapData, (address, bytes));

            if (router == zeroxRouter) {
                ZeroxRouterData memory data = abi.decode(routerData, (ZeroxRouterData));
                // approve needed amount
                SafeERC20.safeApprove(params.tokenIn, data.allowanceTarget, params.amountIn);
                // execute swap
                (bool success,) = zeroxRouter.call(data.data);
                if (!success) {
                    revert SwapFailed();
                }
                // reset approval
                SafeERC20.safeApprove(params.tokenIn, data.allowanceTarget, 0);
            } else if (router == universalRouter) {
                UniversalRouterData memory data = abi.decode(routerData, (UniversalRouterData));
                // tokens are transfered to Universalrouter directly (data.commands must include sweep action!)
                SafeERC20.safeTransfer(params.tokenIn, universalRouter, params.amountIn);
                IUniversalRouter(universalRouter).execute(data.commands, data.inputs, data.deadline);
            } else {
                revert WrongContract();
            }

            uint256 balanceInAfter = params.tokenIn.balanceOf(address(this));
            uint256 balanceOutAfter = params.tokenOut.balanceOf(address(this));

            amountInDelta = balanceInBefore - balanceInAfter;
            amountOutDelta = balanceOutAfter - balanceOutBefore;

            // amountMin slippage check
            if (amountOutDelta < params.amountOutMin) {
                revert SlippageError();
            }

            // event for any swap with exact swapped value
            emit Swap(address(params.tokenIn), address(params.tokenOut), amountInDelta, amountOutDelta);
        }
    }

Tools Used

Manual Review

Recommended Mitigation Steps

The _routerSwap function should check whether the amount of tokens received from the swap is equal to the expected amount of tokens, and revert the transaction if it is not. Here is an example of how this can be done:

function _routerSwap(RouterSwapParams memory params)
    internal
    returns (uint256 amountInDelta, uint256 amountOutDelta)
{
    if (params.amountIn != 0 && params.swapData.length != 0 && address(params.tokenOut) != address(0)) {
        uint256 balanceInBefore = params.tokenIn.balanceOf(address(this));
        uint256 balanceOutBefore = params.tokenOut.balanceOf(address(this));

        // get router specific swap data
        (address router, bytes memory routerData) = abi.decode(params.swapData, (address, bytes));

        if (router == zeroxRouter) {
            ZeroxRouterData memory data = abi.decode(routerData, (ZeroxRouterData));
            // approve needed amount
            SafeERC20.safeApprove(params.tokenIn, data.allowanceTarget, params.amountIn);
            // execute swap
            (bool success,) = zeroxRouter.call(data.data);
            if (!success) {
                revert SwapFailed();
            }
            // reset approval
            SafeERC20.safeApprove(params.tokenIn, data.allowanceTarget, 0);
        } else if (router == universalRouter) {
            UniversalRouterData memory data = abi.decode(routerData, (UniversalRouterData));
            // tokens are transfered to Universalrouter directly (data.commands must include sweep action!)
            SafeERC20.safeTransfer(params.tokenIn, universalRouter, params.amountIn);
            IUniversalRouter(universalRouter).execute(data.commands, data.inputs, data.deadline);
        } else {
            revert WrongContract();
        }

        uint256 balanceInAfter = params.tokenIn.balanceOf(address(this));
        uint256 balanceOutAfter = params.tokenOut.balanceOf(address(this));

        amountInDelta = balanceInBefore - balanceInAfter;
        amountOutDelta = balanceOutAfter - balanceOutBefore;

        // check that the swap returned the expected amount of tokens
        if (amountOutDelta != params.amountOut) {
            revert SwapReturnedUnexpectedAmount();
        }

        // amountMin slippage check
        if (amountOutDelta < params.amountOutMin) {
            revert SlippageError();
        }

        // event for any swap with exact swapped value
        emit Swap(address(params.tokenIn), address(params.tokenOut), amountInDelta, amountOutDelta);
    }
}

The SwapReturnedUnexpectedAmount error can be defined as follows:

error SwapReturnedUnexpectedAmount();

Result in a loss of funds for the user if the swap returns less tokens than expected. Therefore, it is important to check whether the swap returned the expected amount of tokens before proceeding with the transaction.