sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

bin2chen - TOFT.exerciseOptionsReceiver may unable to Retrieve TapToken #92

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

bin2chen

medium

TOFT.exerciseOptionsReceiver may unable to Retrieve TapToken

Summary

In the TOFT.exerciseOptionsReceiver function, if the parameter withdrawOnOtherChain is set to true, the current implementation performs cross-chain transfers of TOFT tokens instead of the expected TapToken.

Vulnerability Detail

In the TOFTOptionsReceiverModule.exerciseOptionsReceiver function, the expected steps are as follows:

  1. Execute ITapiocaOptionBroker.exerciseOption(), paying with paymentToken to obtain TapToken.
  2. If withdrawOnOtherChain is specified, transfer the TapToken obtained in the first step to _options.from on the other chain.

However, in the current implementation, during the second step, the method _sendPacket(msg_.lzSendParams, msg_.composeMsg, _options.from) is called for cross-chain transfer. this method transfers TOFT tokens, not TapToken.

the codes as follows:

contract TOFTOptionsReceiverModule is BaseTOFT {

    function exerciseOptionsReceiver(address srcChainSender, bytes memory _data) public payable {
...
            ITapiocaOptionBroker(_options.target).exerciseOption(
                _options.oTAPTokenID,
                address(this), //payment token
                _options.tapAmount
            );
....

            if (msg_.withdrawOnOtherChain) {
                /// @dev determine the right amount to send back to source
                uint256 amountToSend = _send.amountLD > _options.tapAmount ? _options.tapAmount : _send.amountLD;
                if (_send.minAmountLD > amountToSend) {
                    _send.minAmountLD = amountToSend;
                }

                // Sends to source and preserve source `msg.sender` (`from` in this case).
@>              _sendPacket(msg_.lzSendParams, msg_.composeMsg, _options.from);

                // Refund extra amounts
                if (_options.tapAmount - amountToSend > 0) {
                    IERC20(tapOft).safeTransfer(_options.from, _options.tapAmount - amountToSend);
                }

As a result, it becomes impossible to perform the actual transfer of TapToken.

Impact

In most cases, it is not possible to successfully cross-chain transfer the TapToken obtained from exerciseOption(). In special scenarios, if the endpoint is maliciously transfer TOFT, the execution of _sendPacket() may succeed, leaving the TapToken locked within the contract.

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/TapiocaZ/contracts/tOFT/modules/TOFTOptionsReceiverModule.sol#L197

Tool used

Manual Review

Recommendation

    function exerciseOptionsReceiver(address srcChainSender, bytes memory _data) public payable {
        // Decode received message.
        ExerciseOptionsMsg memory msg_ = TOFTMsgCodec.decodeExerciseOptionsMsg(_data);
...

            address tapOft = ITapiocaOptionBroker(_options.target).tapOFT();
            if (msg_.withdrawOnOtherChain) {
                /// @dev determine the right amount to send back to source
                uint256 amountToSend = _send.amountLD > _options.tapAmount ? _options.tapAmount : _send.amountLD;
                if (_send.minAmountLD > amountToSend) {
                    _send.minAmountLD = amountToSend;
                }

                // Sends to source and preserve source `msg.sender` (`from` in this case).
-               _sendPacket(msg_.lzSendParams, msg_.composeMsg, _options.from);
+               IOftSender(tapOft).sendPacket{value:msg.value}(msg_.lzSendParams, msg_.composeMsg);
                // Refund extra amounts
                if (_options.tapAmount - amountToSend > 0) {
                    IERC20(tapOft).safeTransfer(_options.from, _options.tapAmount - amountToSend);
                }
            } else {
                //send on this chain
                IERC20(tapOft).safeTransfer(_options.from, _options.tapAmount);
            }
        }
    }

Duplicate of #125

sherlock-admin3 commented 5 months ago

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

WangAudit commented:

I don't udnerstand why it will transfer toft instead of expected tap token (don't see it in the code)