Fees is deducted twice during native token wrapping

Summary

During the wrapping of a native token with the wrap function on the mTOFT contract, more than needed value is spent which can cause DOS and incorrect accounting on the mTOFT contract though the value will be present in the vault contract.

Vulnerability Detail

Calling the wrap function found here with native token ie erc20 is address (0) https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/mTOFT.sol#L287 . It is expected to be called with some msg.value as it is a payable function and wants the msg.value amount wrapped. The msg.value is now part of the TOFT contract balance.

The _checkAndExtractFees internal function is called to extract the feeAmount as seen here https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/mTOFT.sol#L300C29-L300C49

In the _checkAndExtractFees function, the vault.registerFees{value: feeAmount}(feeAmount) is called under the conditions that feeAmount is > 0, mintFee > 0 and _getChainId() != hostEid. This means that a portion of the balance of the TOFT contract is used to call vault.registerFees and feeAmount is sent to the vault as seen in https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/mTOFT.sol#L417

_wrapNative function is inherited from the BaseTOFT contract and this internal function is called https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/BaseTOFT.sol#L78

In the _wrapNative the amount is deposited in the vault and wrapped token is minted like vault.depositNative{value: _amount}(); _mint(_toAddress, _amount - _feeAmount); https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/BaseTOFT.sol#L79

Calculating, for an amount input say 1eth, and mintFee of 0.5%, 0.005 eth is first transferred to the vault from vault.registerFees{value: feeAmount}(feeAmount), then 1eth is transferred with depositNative function, and 0.995weth is minted though the _mint function.

This causes the mTOFT contract to lose 1.005 eth to the vault through deposit instead of 1eth for 0.995weth minted This logic is different for non-native tokens, 1 ethEquiv is deposited to the vault and 0.995 worth of wrapped tokens is gotten back, which indicates logic incompatibilty. In the non-native token wrapping logic, 1 eth is sent to the vault and 0.995weth is minted with the feeAmount already taken care of.

Impact

Medium impact, as it can cause DOS if the balance of the TOFT contract at a time is less than amount+feeAmount deposited instead of assumed amount it will cause a revert.

It can also cause incorrect accounting as every time native tokens are wrapped, double the feeAmount is paid, which means an extra feeAmount is sent to the vault during registration and another deducted during wrapping, making the accounted native token balance of the mTOFT contract drop by feeAmount on each wrap call and the _fees recorded by TOFTVault less than received, and cause unexpected issue for the contracts, though the fees can always be transferred back through the transferFees function to revert this issue.

Code Snippet

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);
    }

    function depositNative() external payable onlyOwner {
        if (!_isNative) revert NotValid();
        if (msg.value == 0) revert ZeroAmount();
    }

Tool used

Manual Review

Recommendation

Register the fee in the vault without sending msg.value as the feeAmount will be deducted during the _wrapNative because _amount - _feeAmount will be minted and _amount deposited, accounting for the _feeAmount added to the vault though the registerFees function.

Duplicate of #146

sherlock-audit / 2024-02-tapioca-judging

John_Femi - Fees is deducted twice during native token wrapping #98