sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

ctf_sec - No slippage protection and deadline check when swapping #144

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ctf_sec

high

No slippage protection and deadline check when swapping

Summary

No slippage protection and deadline check when swapping

Vulnerability Detail

the swap function has no slippage protection and deadline check

    /**
     * @notice Swaps tokens
     * @param tokenIn The address of the token to be spent
     * @param tokenOut The address of the token to be obtained
     * @param amountType The type of the amount
     * @param amount When `amountType` is `In`, it's the number of `tokenIn` that the user wants to spend.
     *               When `amountType` is `Out`, it's the number of `tokenOut` that the user wants to obtain.
     * @return amountIn The amount of `tokenIn` spent
     * @return amountOut The amount of `tokenOut` obtained
     */
    function swap(address tokenIn, address tokenOut, AmountType amountType, uint256 amount)
        external
        whenNotPaused
        nonReentrant
        returns (uint256 amountIn, uint256 amountOut)
    {
        IERC20Token feeToken;
        uint256 fee;
        uint24 feeNumerator;
        uint256 price;
        ITokenManager.PairConfig memory pair = tokenManager.getPair(tokenIn, tokenOut);

        // @audit
        // slippage
        (amountIn, amountOut, feeToken, fee, feeNumerator, price) = _getSwapResult(pair, tokenIn, tokenOut, amountType, amount);

        _require(IERC20(tokenIn).balanceOf(msg.sender) >= amountIn, Errors.BALANCE_INSUFFICIENT);

        _swapIn(tokenIn, msg.sender, amountIn);

        _swapOut(tokenOut, msg.sender, amountOut);

        if (fee > 0) {
            address feeReceiver = surplusPool;
            feeToken.mint(feeReceiver, fee);
            emit SwapFeeSent(address(feeToken), feeReceiver, fee);
        }

        _checkReserveRatio(tokenOut == pair.baseToken ? pair.buyReserveRatioThreshold : pair.sellReserveRatioThreshold);

        emit Swapped(tokenIn, tokenOut, msg.sender, amountIn, amountOut, address(feeToken), fee, feeNumerator, price);
    }

no slippage protection means the user is subject to frontrunning

_swapIn(tokenIn, msg.sender, amountIn);

_swapOut(tokenOut, msg.sender, amountOut);

no deadline check means the transaction can be pending in the mempool for a very long time and the oracle is updated multiple times and the price / exchange already changes and combing with there is no slippage control, user can receive very suboptimal amount

Impact

No slippage protection and deadline check when swapping

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/d5328421bea80e3b0fd4595e4eb6b732a40e421e/Unitas-Protocol/src/Unitas.sol#L208

Tool used

Manual Review

Recommendation

Add minAmount receive and deadline check just like other AMM pool

Duplicate of #88