Legitimate Wrap Transactions Could Erroneously Be Reverted

Summary

Erroneous use of the input amount as the expected minted amount will cause legitimate wrap txns to revert in mTOFT::wrap

Vulnerability Detail

mTOFT::wrap function is a functionality used for wrapping the underlying token for the contract oft token. The supported underlying token can either be an erc20 or a native token. When wrapping, the functionality ensures that the total supply of its token is always below the set mint cap.

        if (mintCap > 0) {
            if (totalSupply() + _amount > mintCap) revert mTOFT_CapNotValid();
        }

The problem here is that, the wrong amount is used to ensure the total supply of the contract oft tokens is below the mint cap.

To Illustrate:

Assuming mintCap = 500 totalSupply = 400 then Bob wants to wrap 110 The fee for wrapping is set to 10% The current implementation does this: totalSupply() + _amount > mintCap = 400 + 110 ==> 510 Since 510 is greater than 500, the call reverts. When actually; fee = 110 * 10% = 11 minted amount = amount - fee = 100 - 11 ==> 89

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

As observed from the internal _wrapNative function, the actual amount minted is the input amount minus the charged fee, thus from our above example: totalSupply() + minted amount = 400 + 89 ==> 489 Since the total supply/minted oft tokens(489) will be less than the mint cap(500), the function shouldn't have reverted

Impact

The incorrect use of the input amount as the minted amount will result in erroneous reverting of transactions, preventing legitimate token wrapping operations.

Code Snippet

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

Tool used

Manual Review

Recommendation

Update the mTOFT::wrap function to:

    function wrap(address _fromAddress, address _toAddress, uint256 _amount)
        external
        payable
        whenNotPaused
        nonReentrant
        returns (uint256 minted)
    {
        if (balancers[msg.sender]) revert mTOFT_BalancerNotAuthorized();
        if (!connectedChains[_getChainId()]) revert mTOFT_NotHost();

        uint256 feeAmount = _checkAndExtractFees(_amount);
+        minted = _amount - feeAmount;
  +      if (mintCap > 0) {
    +        if (totalSupply() + minted > mintCap) revert mTOFT_CapNotValid();
      +  }

        if (erc20 == address(0)) {
            _wrapNative(_toAddress, _amount, feeAmount);
        } else {
            if (msg.value > 0) revert mTOFT_NotNative();
            _wrap(_fromAddress, _toAddress, _amount, feeAmount);
        }
    }

Duplicate of #138

cryptotechmaker commented 7 months ago

It purposely includes the fee

sherlock-admin3 commented 7 months ago

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

WangAudit commented:

I believe it's design decision and seems just like extra caution

sherlock-audit / 2024-02-tapioca-judging

Tendency - Legitimate Wrap Transactions Could Erroneously Be Reverted #20