bin2chen - mTOFT when erc20==address(0) need to pay fees twice

[sherlock-admin2]

bin2chen

medium

mTOFT when erc20==address(0) need to pay fees twice

Summary

In mTOFT.wrap() Due to incorrect implementation, need to pay fees twice to succeed

Vulnerability Detail

In mTOFT.wrap(), when erc20==address(0), the payment process is as follows:

1. Execute _checkAndExtractFees():
   • Will execute vault.registerFees{value: feeAmount}(feeAmount); -> pay fees
2. Execute vault.depositNative{value: _amount}(); -> also contain fees
3. _mint(_toAddress, _amount - _feeAmount);

The first step has already paid fees, the second step amount still contains fees

the codes as follows:

    function wrap(address _fromAddress, address _toAddress, uint256 _amount)
        external
        payable
        whenNotPaused
        nonReentrant
        returns (uint256 minted)
    {
...

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

    function _checkAndExtractFees(uint256 _amount) private returns (uint256 feeAmount) {
        feeAmount = 0;

        // not on host chain; extract fee
        // fees are used to rebalance liquidity to host chain
        if (_getChainId() != hostEid && mintFee > 0) {
            feeAmount = (_amount * mintFee) / 1e5;
            if (feeAmount > 0) {
                if (erc20 == address(0)) {
@>                  vault.registerFees{value: feeAmount}(feeAmount);
                } else {
                    vault.registerFees(feeAmount);
                }
            }
        }
    }

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

As mentioned above, suppose deposit _amount = 10, _feeAmount = 2 In the end:

1. msg.value need = 2 + 10 =12
2. but mint(10-2) = mint(8)

pay 4 fees

Impact

when erc20==address(0) need to pay fees twice

Code Snippet

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

Tool used

Manual Review

Recommendation

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

Duplicate of #146

[sherlock-admin4]

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

WangAudit commented:

I checked this on RemixIDE and actually the call to vault.depositNative will not send the msg.value cause it has decreased; therefore; the _amount is bigger than msg.value (after sending fees); also I tested and depositNative won't revert on msg.value == 0 check

takarez commented:

invalid; see 023

[dmitriia]

That's valid: msg.value doesn't change on sending gas tokens out, it is a fixed incoming value for any function. Native tokens wrap is a core feature of the protocol, it's intended to be used with erc20 == address(0).