hyh - TOFT and mTOFT wrapping and executor swaps don't control for `msg.value` when deal with gas tokens, so any excess native token funds can be immediately stolen by anyone via back-running

[sherlock-admin4]

hyh

medium

TOFT and mTOFT wrapping and executor swaps don't control for msg.value when deal with gas tokens, so any excess native token funds can be immediately stolen by anyone via back-running

Summary

In the course of native token operations msg.value isn't controlled to match amount, so all the native tokens send over by users in excess of amount, i.e. msg.value - amount, can be stolen by a back-running attacker (who specify their amount < msg.value).

Vulnerability Detail

Residue native funds from TOFT, mTOFT and executor contract balances can be stolen.

Particularly, TOFT and mTOFT wrapping are directly user-facing, so the excess funds occurrences can be expected in general. There is also a probability of user mistakes, when msg.value - amount isn't a residue, but something material, being a result of user operational mistake. Attacker can setup a bot, automatically tracking all such events, immediately extracting these funds with back-running whenever the expected result exceeds gas costs.

Impact

The remainder funds from mentioned operations can be stolen by the next caller. The prerequisite is user supplying more than amount, so the probability is low (but not very low as the operations are user facing). Funds stealing impact has high severity.

Likelihood: Low + Impact: High = Severity: Medium.

Code Snippet

msg.value isn't controlled to match amount in many instances, e.g.:

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/mTOFT.sol#L287-L303

    function wrap(address _fromAddress, address _toAddress, uint256 _amount)
        ...
    {
        if (balancers[msg.sender]) revert mTOFT_BalancerNotAuthorized();
        if (!connectedChains[_getChainId()]) revert mTOFT_NotHost();
        if (mintCap > 0) {
            if (totalSupply() + _amount > mintCap) revert mTOFT_CapNotValid();
        }

        uint256 feeAmount = _checkAndExtractFees(_amount);
        if (erc20 == address(0)) {
>>          _wrapNative(_toAddress, _amount, feeAmount);
        } else {

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/TOFT.sol#L242-L251

    function wrap(address _fromAddress, address _toAddress, uint256 _amount)
        external
        payable
        whenNotPaused
        nonReentrant
        onlyHostChain
        returns (uint256 minted)
    {
        if (erc20 == address(0)) {
>>          _wrapNative(_toAddress, _amount, 0);

No amount information is left at this point:

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/BaseTOFT.sol#L78-L81

    function _wrapNative(address _toAddress, uint256 _amount, uint256 _feeAmount) internal virtual {
>>      vault.depositNative{value: _amount}();
        _mint(_toAddress, _amount - _feeAmount);
    }

Similarly, executor's getCollateral() and getAsset() don't ensure that msg.value is being utilized fully, e.g.:

function getCollateral(
    address assetAddress,
    address collateralAddress,

 uint256 assetAmountIn,

bytes calldata swapperData ) external payable override returns (uint256 collateralAmountOut) { // Should be called only by approved SGL/BB markets. if (!cluster.isWhitelisted(0, msg.sender)) revert SenderNotValid(); return _swapAndTransferToSender(true, assetAddress, collateralAddress, assetAmountIn, swapperData); }

As assetAmountIn = amountIn is used instead:

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/leverage/BaseLeverageExecutor.sol#L129-L147

    function _swapAndTransferToSender(
        bool sendBack,
        address tokenIn,
        address tokenOut,
>>      uint256 amountIn,
        bytes memory data
    ) internal returns (uint256 amountOut) {
        SLeverageSwapData memory swapData = abi.decode(data, (SLeverageSwapData));

        // If the tokenIn is a tOFT, unwrap it. Handles ETH and ERC20.
        if (swapData.toftInfo.isTokenInToft) {
>>          tokenIn = _handleToftUnwrap(tokenIn, amountIn);
        }

        // Approve the swapper to spend the tokenIn, and perform the swap.
>>      tokenIn.safeApprove(address(swapper), amountIn);
        IZeroXSwapper.SZeroXSwapData memory swapperData =
            abi.decode(swapData.swapperData, (IZeroXSwapper.SZeroXSwapData));
        amountOut = swapper.swap(swapperData, amountIn, swapData.minAmountOut);

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/markets/leverage/BaseLeverageExecutor.sol#L169-L179

    function _handleToftUnwrap(address tokenIn, uint256 amountIn) internal returns (address tokenToSwap) {
        ITOFT(tokenIn).unwrap(address(this), amountIn); // Sends ETH to `receive()` if not an ERC20.
        tokenIn = ITOFT(tokenIn).erc20();
        // If the tokenIn is ETH, wrap it to WETH.
        if (tokenIn == address(0)) {
>>          weth.deposit{value: amountIn}();
            tokenToSwap = address(weth);
        } else {
            tokenToSwap = tokenIn;
        }
    }

Tool used

Manual Review

Recommendation

Consider adding the amount checks for the native case, e.g.:

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/mTOFT.sol#L287-L303

    function wrap(address _fromAddress, address _toAddress, uint256 _amount)
        ...
    {
        if (balancers[msg.sender]) revert mTOFT_BalancerNotAuthorized();
        if (!connectedChains[_getChainId()]) revert mTOFT_NotHost();
        if (mintCap > 0) {
            if (totalSupply() + _amount > mintCap) revert mTOFT_CapNotValid();
        }

        uint256 feeAmount = _checkAndExtractFees(_amount);
        if (erc20 == address(0)) {
+           if (msg.value != _amount) revert mTOFT_AmountMismatch();
            _wrapNative(_toAddress, _amount, feeAmount);
        } else {

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/mTOFT.sol#L93-L98

    error mTOFT_NotNative();
    error mTOFT_NotHost();
    error mTOFT_BalancerNotAuthorized();
    error mTOFT_NotAuthorized();
    error mTOFT_CapNotValid();
    error mTOFT_Failed();
+   error mTOFT_AmountMismatch();

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/TOFT.sol#L242-L251

    function wrap(address _fromAddress, address _toAddress, uint256 _amount)
        external
        payable
        whenNotPaused
        nonReentrant
        onlyHostChain
        returns (uint256 minted)
    {
        if (erc20 == address(0)) {
+           if (msg.value != _amount) revert TOFT_AmountMismatch();
            _wrapNative(_toAddress, _amount, 0);

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/TOFT.sol#L47-L50

contract TOFT is BaseTOFT, Pausable, ReentrancyGuard, ERC20Permit {
    error TOFT_OnlyHostChain();
    error TOFT_NotNative();
    error TOFT_Failed();
+   error TOFT_AmountMismatch();

Duplicate of #132

[sherlock-admin2]

1 comment(s) were left on this issue during the judging contest.

WangAudit commented:

basically user mistake -> invalid under Sherlock's rules