TOFT exerciseOption can be used to steal all underlying erc20 tokens

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/modules/BaseTOFTOptionsModule.sol#L255

Vulnerability details

Impact

Unvalidated input data for the exerciseOption function can be used to steal all the erc20 tokens from the contract.

Proof of Concept

Each BaseTOFT is a wrapper around an erc20 token and extends the OFTV2 contract to enable smooth cross-chain transfers through LayerZero.
Depending on the erc20 token which is used usually the erc20 tokens will be held on one chain and then only the shares of OFTV2 get transferred around (burnt on one chain, minted on another chain).
Subject to this attack is TapiocaOFTs or mTapiocaOFTs which store as an underlying token an erc20 token(not native). In order to mint TOFT shares you need to deposit the underlying erc20 tokens into the contract, and you get TOFT shares.

The attack flow is the following:

1. The attack starts from the exerciseOption. Nothing is validated here and the only cost of the attack is the optionsData.paymentTokenAmount which is burned from the attacker. This can be some small amount.
2. When the message is received on the remote chain inside the exercise function it is important that nothing reverts for the attacker.
3. For the attacker to go through the attacker needs to pass the following data:

function exerciseInternal(
        address from,
        uint256 oTAPTokenID,
        address paymentToken,
        uint256 tapAmount,
        address target,
        ITapiocaOptionsBrokerCrossChain.IExerciseLZSendTapData
            memory tapSendData,
        ICommonData.IApproval[] memory approvals
    ) public {
        // pass zero approval so this is skipped 
        if (approvals.length > 0) {
            _callApproval(approvals);
        }
        
        // target is the address which does nothing, but has the exerciseOption implemented
        ITapiocaOptionsBroker(target).exerciseOption(
            oTAPTokenID,
            paymentToken,
            tapAmount
        );
        // tapSendData.withdrawOnAnotherChain = false so we enter else branch
        if (tapSendData.withdrawOnAnotherChain) {
            ISendFrom(tapSendData.tapOftAddress).sendFrom(
                address(this),
                tapSendData.lzDstChainId,
                LzLib.addressToBytes32(from),
                tapAmount,
                ISendFrom.LzCallParams({
                    refundAddress: payable(from),
                    zroPaymentAddress: tapSendData.zroPaymentAddress,
                    adapterParams: LzLib.buildDefaultAdapterParams(
                        tapSendData.extraGas
                    )
                })
            );
        } else {
            // tapSendData.tapOftAddress is the address of the underlying erc20 token for this TOFT
            // from is the address of the attacker
            // tapAmount is the balance of erc20 tokens of this TOFT
            IERC20(tapSendData.tapOftAddress).safeTransfer(from, tapAmount);
        }
    }

4. So the attack is just simply transferring all the underlying erc20 tokens to the attacker.

The underlying ERC20 token for each TOFT can be queried through erc20() function, and the tapAmount to pass is ERC20 balance of the TOFT.

This attack is possible because the msg.sender inside the exerciseInternal is the address of the TOFT which is the owner of all the ERC20 tokens that get stolen.

Tools Used

• Manual review

Recommended Mitigation Steps

Validate that tapSendData.tapOftAddress is the address of TapOFT token either while sending the message or during the reception of the message on the remote chain.

Assessed type

Token-Transfer

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-sponsor]

0xRektora (sponsor) confirmed

[c4-judge]

dmvt marked the issue as selected for report