Exercising Options In a destination Chain for Some msg Type is Impossible

Summary

Composing a message to exercise options in a destination chain breaks whenever withdrawOnOtherChain is set to true, due to how the contract currently attempts to burn from a wrong address.

Vulnerability Detail

When sending messages across different chains, composed messages can be packed along the forwarded payload to be executed on the destination chain in layer zero v2. The described issue here is for when the composed message is intended to invoke the destination chain TOFTOptionsReceiverModule::exerciseOptionsReceiver function The call route will be as follows:

mTOFT::sendPacket --> _executeModule --> TapiocaOmnichainSender::sendPacket --> _lzSend -->EndpointV2::send --> mTOFT::lzReceive --> executeModule -->OAppReceiver::lzReceive --> TapiocaOmnichainReceiver::_lzReceive --> EndpointV2::sendCompose --> TapiocaOmnichainReceiver::lzCompose --> _lzCompose --> BaseTOFTReceiver::_toeComposeReceiver --> TOFTOptionsReceiverModule::exerciseOptionsReceiver

From the call path, we can observe that, when there is a composed message, the Layer Zero EndpointV2::sendCompose function is called, and down this function execution, Layer Zero calls back to the calling contract(mTOFT in this case) lzCompose function. For our desired message type, the internal function _toeComposeReceiver is called which then goes on via _executeModule to make a delegate call to TOFTOptionsReceiverModule::exerciseOptionsReceiver function. Note that, the msg.sender here will still be preserved, and thus remains the layer zero endpoint address.

The exerciseOptionsReceiver function is intended to allow users to send in an amount of mTOFT tokens to receive a certain amount of tapOFT tokens. When withdrawing in the destination chain, the received tapOft tokens are sent to the input options receiver

       } else {
                //send on this chain
                IERC20(tapOft).safeTransfer(_options.from, _options.tapAmount);
            }

The problem here is when the user intends to withdraw on another chain, for this message type, the internal function _sendPacket is called to send the message along to the chosen destination chain. In _sendPacket, the internal _debit function is called to burn an amount of the contract oft tokens, which will then be minted to the user in the destination chain. Here's the _debit function:

    function _debit(
        uint256 _amountLD,
        uint256 _minAmountLD,
        uint32 _dstEid
    ) internal virtual override returns (uint256 amountSentLD, uint256 amountReceivedLD) {
        (amountSentLD, amountReceivedLD) = _debitView(_amountLD, _minAmountLD, _dstEid);

        // @dev In NON-default OFT, amountSentLD could be 100, with a 10% fee, the amountReceivedLD amount is 90,
        // therefore amountSentLD CAN differ from amountReceivedLD.

        // @dev Default OFT burns on src.
        _burn(msg.sender, amountSentLD); // <--@
    }

Notice the default implementation attempts to burn from the msg.sender. As we have previously noted, remember the sender in this context is currently the lz endpoint and not the user, this will thus result in a revert since the endpoint wouldn't have any contract oft tokens to burn.

Impact

Exercising options in a destination chain, to then receive these options on another chain will always result in a revert

Code Snippet

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

Tool used

Manual Review

Recommendation

It is unclear what the expected behaviour here is, since even if the right user is being debited, the code will still be broken. The reason being:

The user has already deposited an amount of the contract oft tokens for tapOft tokens https://github.com/sherlock-audit/2024-02-tapioca/blob/dc2464f420927409a67763de6ec60fe5c028ab0e/TapiocaZ/contracts/tOFT/modules/TOFTOptionsReceiverModule.sol#L155-L156
The contract has already received these tap tokens https://github.com/sherlock-audit/2024-02-tapioca/blob/dc2464f420927409a67763de6ec60fe5c028ab0e/TapiocaZ/contracts/tOFT/modules/TOFTOptionsReceiverModule.sol#L166-L170
If for example, the user has oft enough to also burn at _debit, the contract keeps the received tapOft, and the user receives the destination chain oft tokens and not tapOft.

With all this, I believe the option to withdraw on another chain should be removed, users should make cross-chain calls only to chains they intend to receive tapoft from instead.

Note that the issue should also be fixed in Tapioca-Bar https://github.com/sherlock-audit/2024-02-tapioca/blob/dc2464f420927409a67763de6ec60fe5c028ab0e/Tapioca-bar/contracts/usdo/modules/UsdoOptionReceiverModule.sol#L113-L126

Duplicate of #125

sherlock-audit / 2024-02-tapioca-judging

Tendency - Exercising Options In a destination Chain for Some msg Type is Impossible #15