sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

duc - Handling the case where `msg_.unwrap` == false is missed in the `TOFTGenericReceiverModule.receiveWithParamsReceiver` function #86

Closed sherlock-admin2 closed 5 months ago

sherlock-admin2 commented 5 months ago

duc

high

Handling the case where msg_.unwrap == false is missed in the TOFTGenericReceiverModule.receiveWithParamsReceiver function

Summary

See Vulnerability Detail

Vulnerability Detail

TOFTGenericReceiverModule.receiveWithParamsReceiver function is used to transfer received tokens to msg_.receiver. However, this function only attempts to transfer tokens in the case where msg_.unwrap is true. If msg_.unwrap is false, it doesn't do anything.

function receiveWithParamsReceiver(address srcChainSender, bytes memory _data) public payable {
    SendParamsMsg memory msg_ = TOFTMsgCodec.decodeSendParamsMsg(_data);

    msg_.amount = _toLD(msg_.amount.toUint64());

    if (msg_.unwrap) {
        ITOFT tOFT = ITOFT(address(this));
        address toftERC20 = tOFT.erc20();

        /// @dev xChain owner needs to have approved dst srcChain `sendPacket()` msg.sender in a previous composedMsg. Or be the same address.
        _internalTransferWithAllowance(msg_.receiver, srcChainSender, msg_.amount);
        tOFT.unwrap(address(this), msg_.amount);

        if (toftERC20 != address(0)) {
            IERC20(toftERC20).safeTransfer(msg_.receiver, msg_.amount);
        } else {
            (bool sent,) = msg_.receiver.call{value: msg_.amount}("");
            if (!sent) revert TOFTGenericReceiverModule_TransferFailed();
        }
    }
}

Impact

Loss of funds for user when using this functionality of TOFT.

Code Snippet

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

Tool used

Manual Review

Recommendation

Should also consider the case where msg_.unwrap is false

Duplicate of #138

cryptotechmaker commented 5 months ago

Invalid; nothing has to be done on the else branch as the tokens are transferred to the user by lzSend

sherlock-admin4 commented 5 months ago

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

WangAudit commented:

seems like design decision; cause function's comments say the function is used to Unwrap and sends underlying to receiver.