Potential zero swap fee in SmartVaultV3::swap function

Severity

Low Risk

Relevant GitHub Links

https://github.com/Cyfrin/2023-12-the-standard/blob/91132936cb09ef9bf82f38ab1106346e2ad60f91/contracts/SmartVaultV3.sol#L215

Summary

The SmartVaultV3::swap function calculates the swapFee based on the input _amount parameter, the swapFeeRate and HUNDRED_PC obtained from the ISmartVaultManagerV3(manager). If the _amount is a small, the swapFee can be 0 due to the rounding down by division.

Vulnerability Details

In the deploy.js script the swapFeeRate is set to 500. The HUNDRED_PC variable is a constant and it is set to 1e5. Therefore, if the _amount parameter is a small (lower than 200), the value of swapFee will be 0:

    function swap(bytes32 _inToken, bytes32 _outToken, uint256 _amount) external onlyOwner {
@>      uint256 swapFee = _amount * ISmartVaultManagerV3(manager).swapFeeRate() / ISmartVaultManagerV3(manager).HUNDRED_PC();
        address inToken = getSwapAddressFor(_inToken);
        uint256 minimumAmountOut = calculateMinimumAmountOut(_inToken, _outToken, _amount);
        ISwapRouter.ExactInputSingleParams memory params = ISwapRouter.ExactInputSingleParams({
                tokenIn: inToken,
                tokenOut: getSwapAddressFor(_outToken),
                fee: 3000,
                recipient: address(this),
                deadline: block.timestamp,
                amountIn: _amount - swapFee,
                amountOutMinimum: minimumAmountOut,
                sqrtPriceLimitX96: 0
            });
        inToken == ISmartVaultManagerV3(manager).weth() ?
            executeNativeSwapAndFee(params, swapFee) :
            executeERC20SwapAndFee(params, swapFee);
    }

Let's consider the value of swapFee if the _amount is 20:

uint256 swapFee = 20 * 500 / 1e5

In that case the value of swapFee will be 0 due to the rounding down by division.

Impact

If the _amount parameter in SmartVaultV3::swap function is lower than 200, the calculated amount for swapFee variable will be 0. That allows the users to swap small amounts without fees. That can be issue for the protocol, because the protocol will not receive fees for small values of _amount.

Additionally, if the SmartVaultV3::swap function calls the SmartVaultV3::executeERC20SwapAndFee function with swapFee parameter equals to 0 and the protocol will use in the future weird ERC20 tokens which revert on zero value transfer (e.g. LEND), the entire transaction will fail, including the swap operation.

   function executeERC20SwapAndFee(ISwapRouter.ExactInputSingleParams memory _params, uint256 _swapFee) private {
@>      IERC20(_params.tokenIn).safeTransfer(ISmartVaultManagerV3(manager).protocol(), _swapFee);
        IERC20(_params.tokenIn).safeApprove(ISmartVaultManagerV3(manager).swapRouter2(), _params.amountIn);
        ISwapRouter(ISmartVaultManagerV3(manager).swapRouter2()).exactInputSingle(_params);
        IWETH weth = IWETH(ISmartVaultManagerV3(manager).weth());
        // convert potentially received weth to eth
        uint256 wethBalance = weth.balanceOf(address(this));
        if (wethBalance > 0) weth.withdraw(wethBalance);

Tools Used

Manual Review

Recommendations

Implement a minimum fee threshold to prevent the fee from being zero. Add validation checks to ensure that the calculated swapFee is greater than zero before proceeding with the swap. While the risk of financial loss due to a zero swapFee is low, it is important to address this issue to ensure that the protocol's fee mechanisms are enforced as intended.