TOFT exerciseOption fails due to not passing msg.value properly

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/BaseTOFT.sol#L127-L146
https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/BaseTOFT.sol#L536-L550

Vulnerability details

Impact

Packet type PT_TAP_EXERCISE always reverts due to not passing properly the msg.value to the final destination contract.
When first sent out it will revert and be stored inside failedMessages and the user is not able to retry the message.
The user losses gas and TOFT tokens by using this function.

Layer Zero message delivery

LayerZero is a messaging protocol that enables the delivery of payload from chainA to chainB.
The sender specifies and pays for the destination gas and has the option of airdropping native gas tokens into an address on the destination chain. This is done through relayer params.
The Relayer invokes the validateTransactionProofV2 function on the destination chain to deliver the payload.
The airdropped value is not part of the msg.value but is delivered as address(this).balance on the destination contract. Also, the validateTransactionProofV2 function doesn't revert if airdropped tokens are not sent to the destination contract. It just emits an event, so the assumption always has to be that nothing is airdropped in the worst case.

(bool sent, ) = _to.call{value: msg.value}("");
//require(sent, "Relayer: failed to send ether");
if (!sent) {
    emit ValueTransferFailed(_to, msg.value);
}

Proof of Concept

The flow of the issue is the following:

1. User invokes exerciseOption function and is allowed to pass on a bunch of parameters which are not validated.
   The only thing which is validated is does he have enough TOFT tokens, which are being burned and the packetType is PT_TAP_EXERCISE and he cannot airdrop any native tokens since adapterParamsV1 are enforced.
2. When this message is delivered to the destination chain, the first step is decoding the params.
3. If one of the decoded params tapSendData.withdrawOnAnotherChain is true after exercising the option the tapOFT tokens are tried to be sent out to another chain. The problem is that the sendFrom is missing {value: address(this.balance) and the transaction reverts.
4. if we take a look at the implementation of the sendFrom function inside the BaseOFTV2 and the underlying _send function in OFTCoreV2.
   We can see that the _lzSend relies on the msg.value. However, in this case msg.value is always 0.
5. Even if the sending side allowed to airdrop some native tokens, again in my explanation of the Layer Zero message delivery the airdropped tokens are always airdropped as a balance of the contract, so the issue needs to be addressed in multiple places in the code.
6. What is troublesome is that once this message is stored inside the failedMessages mapping, the user can never retry it because it would always fail due to the same reason of not having sendFrom{value: address(this).balance}.

Tools Used

• Manual review
• Foundry
• LayerZero scan

Recommended Mitigation Steps

In my other issue, I have elaborated on the issues of airdropping users' tokens into TOFT's contract balance. It should be avoided. But the immediate fix for this issue is:

• Add the option of airdropping tokens into the exerciseOption through adapterParamsV2 as it is done for many other packet types.

• Add the value to the sendFrom external call.

ISendFrom(tapSendData.tapOftAddress).sendFrom{value: address(this).balance}(
                address(this),
                tapSendData.lzDstChainId,
                LzLib.addressToBytes32(from),
                tapAmount,
                ISendFrom.LzCallParams({
                    refundAddress: payable(from),
                    zroPaymentAddress: tapSendData.zroPaymentAddress,
                    adapterParams: LzLib.buildDefaultAdapterParams(
                        tapSendData.extraGas
                    )
                })
            );

Other occurrences

• This exact same issue occurs while exercising option through BaseUSDO and when it is received on the other chain through the exercise function. This would also result in loss of USDO tokens and gas for the user.

• initMultiHopBuy results in loss of airdropped tokens since no USDO is burned in this case.
  This function offers the option of airdropping tokens through airdropAdapterParams but when it is received on the remote chain it invokes multiHopBuyCollateral on the Singularity contract which expects msg.value.
  Thus, the whole thing would revert and be saved inside failedMessages mapping. And since the {value: address(this).balance} is missing in the multiHop the user cannot retry the message.

• initMultiSell suffers from the same issue I just described above. The user would lose gas and airdropped tokens. {value: address(this).balance} is missing from multiHopSellCollateral function.

Assessed type

Payable

[c4-pre-sort]

minhquanym marked the issue as duplicate of #1199

[c4-judge]

dmvt changed the severity to 2 (Med Risk)

[c4-judge]

dmvt marked the issue as selected for report

[c4-sponsor]

0xRektora (sponsor) confirmed