sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

0xadrii - Missing return statement will make mtOFT's compose calls of message type MSG_XCHAIN_LEND_XCHAIN_LOCK always fail #122

Closed sherlock-admin2 closed 6 months ago

sherlock-admin2 commented 6 months ago

0xadrii

high

Missing return statement will make mtOFT's compose calls of message type MSG_XCHAIN_LEND_XCHAIN_LOCK always fail

Summary

Missing a return statement when executing compose calls of type MSG_XCHAIN_LEND_XCHAIN_LOCK will lead to the call always failing, preventing these type of calls from ever taking place.

Vulnerability Detail

When a compose message is received in an mtOFT token, TapiocaOmnichainReceiver’s _lzCompose() function will be executed. This function will handle different kinds of msgType, prioritizing MSG_REMOTE_TRANSFER and the messages checked inside _extExec():

// TapiocaOmnichainReceive.sol

function _lzCompose(address srcChainSender_, bytes32 _guid, bytes memory oftComposeMsg_) internal {
        // Decode OFT compose message.
        (uint16 msgType_,,, bytes memory tapComposeMsg_, bytes memory nextMsg_) =
            TapiocaOmnichainEngineCodec.decodeToeComposeMsg(oftComposeMsg_);

        // Call Permits/approvals if the msg type is a permit/approval.
        // If the msg type is not a permit/approval, it will call the other receivers.
        if (msgType_ == MSG_REMOTE_TRANSFER) {
            _remoteTransferReceiver(srcChainSender_, tapComposeMsg_);
        } else if (!_extExec(msgType_, tapComposeMsg_)) {
            ...
            } else {
                // If no TOE extender is set or msg type doesn't match extender, try to call the internal receiver.
                if (!_toeComposeReceiver(msgType_, srcChainSender_, tapComposeMsg_)) {
                    revert InvalidMsgType(msgType_);
                }
            }
        }

        emit ComposeReceived(msgType_, _guid, tapComposeMsg_);
        if (nextMsg_.length > 0) {
            _lzCompose(address(this), _guid, nextMsg_);
        }
    }

As the code snippet shows, the last message type check and execution will be performed inside the _toeComposeReceiver(), which contains message type checks specific to the token (in this case, the mtOFT). The _toeComposeReceiver() implementation can be found inside BaseTOFTReceiver.sol. This function will check another set of message types (hidden in the … in the next code snippet for simplicity), and if no message type is found, then a final _toftCustomComposeReceiver() will be called.

// BaseTOFTReceiver.sol

function _toeComposeReceiver(uint16 _msgType, address _srcChainSender, bytes memory _toeComposeMsg)
        internal
        override
        returns (bool success)
    {
        ... 
        } else {
            return _toftCustomComposeReceiver(_msgType, _srcChainSender, _toeComposeMsg);
        }
        return true; 
    }

For mtOFTs, the _toftCustomComposeReceiver() is implemented in mtOFTReceiver.sol:

// mtOFTReceiver.sol
function _toftCustomComposeReceiver(uint16 _msgType, address, bytes memory _toeComposeMsg)
        internal
        override
        returns (bool success)
    {
        if (_msgType == MSG_LEVERAGE_UP) {
            _executeModule(
                uint8(ITOFT.Module.TOFTMarketReceiver),
                abi.encodeWithSelector(TOFTMarketReceiverModule.leverageUpReceiver.selector, _toeComposeMsg),
                false
            );
            return true;
        } else if (_msgType == MSG_XCHAIN_LEND_XCHAIN_LOCK) {
            _executeModule( 
                uint8(ITOFT.Module.TOFTOptionsReceiver),
                abi.encodeWithSelector(
                    TOFTOptionsReceiverModule.mintLendXChainSGLXChainLockAndParticipateReceiver.selector, _toeComposeMsg
                ),
                false
            ); 
        } else {
            return false;
        }
    }

The issue with this function is that when _msgType is of type MSG_XCHAIN_LEND_XCHAIN_LOCK , a return value of true should be sent, but as the code snippet shows, nothing is returned instead. This will make a false success value be returned due to Solidity defaulting booleans to false.

Because a false return value is sent (even if the message type is correct), the if condition performed inside the _lzCompose() which checks whether _toeComposeReceiver() returns a false value will evaluate to true, making messages of type MSG_XCHAIN_LEND_XCHAIN_LOCK always revert with an invalid InvalidMsgType() error.

Impact

High. One of mtOFT ’s core functionalities is broken, making it impossible to execute compose calls that trigger the TOFTOptionsReceiver module mintLendXChainSGLXChainLockAndParticipateReceiver() function.

Code Snippet

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

Tool used

Manual Review

Recommendation

Return a true value inside _toftCustomComposeReceiver() when message is of type MSG_XCHAIN_LEND_XCHAIN_LOCK:

// mtOFTReceiver.sol
function _toftCustomComposeReceiver(uint16 _msgType, address, bytes memory _toeComposeMsg)
        internal
        override
        returns (bool success)
    {
        if (_msgType == MSG_LEVERAGE_UP) {
            _executeModule(
                uint8(ITOFT.Module.TOFTMarketReceiver),
                abi.encodeWithSelector(TOFTMarketReceiverModule.leverageUpReceiver.selector, _toeComposeMsg),
                false
            );
            return true;
        } else if (_msgType == MSG_XCHAIN_LEND_XCHAIN_LOCK) {
            _executeModule( 
                uint8(ITOFT.Module.TOFTOptionsReceiver),
                abi.encodeWithSelector(
                    TOFTOptionsReceiverModule.mintLendXChainSGLXChainLockAndParticipateReceiver.selector, _toeComposeMsg
                ),
                false
            ); 
+            return true;
        } else {
            return false;
        }
    }

Duplicate of #63