Messages revert on the receiving chain and user can't get his funds back

[code423n4]

Lines of code

https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/modules/BaseTOFTLeverageModule.sol#L205-L267
https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/master/contracts/usd0/modules/USDOLeverageModule.sol#L190-L252

Vulnerability details

Impact

This is an issue that affects BaseUSDO and BaseTOFT through various message/packet types since they delegatecall into various modules and all the complex execution is wrapped in a big try/catch block, so if any part of the logic fails the whole message gets stored in the failedMessages.
The impact is that user cannot access his TOFT tokens in cases where the failed message retry keeps on reverting due to the changed state of the system.
This is most likely to occur in cases where the passed data on the home chain can get outdated pretty quickly, which is the case in sendForLeverage since there is swapping on a DEX involved.

Proof of Concept

I will illustrate the issue on a single example.

1. A user invokes the sendForLeverage function on the BaseTOFT contract.
2. His message is received on the remote chain inside the lzReceive and the logic of leverageDown is being executed.
3. After decoding the payload TOFT shares are temporarily credited(minted) to the address of the TOFT.
4. After that there is a big piece of logic wrapped in try catch
5. The logic inside leverageInternal can fail due to various reasons.
6. It takes (1-5 min or more) for the Relayer to deliver a message to the remote, so swap data can get outdated, and the airdropped value can be not enough to cover the gas costs of the message execution on the remote chain, etc.
7. In case something fails this is handled by reverting the whole execution
8. First of all this part of code and also the transfer
   are completely useless since the whole function is reverted anyway.
9. The core of the issue lies in the fact that this revert causes the message to be stored inside failedMessages and it can only be retried with the exact same payload.
10. As a consequence if the user tries to retry it will just keep on reverting, and he will never be able to get his TOFT shares back.

The exact same issue is present in BaseUSDO's leveraging logic.
But also anywhere in the code whereby there is a revert and the user is not getting his shares back. I've noted all the occurrences of this issue at the end of the report.

Tools Used

• Manual review

Recommended Mitigation Steps

As an immediate solution in case the whole delegatecall has failed just transfer the TOFT tokens to the user without reverting.

if (!success) {
    if (balanceAfter - balanceBefore >= borrowParams.amount) {
        IERC20(address(this)).safeTransfer(_from, borrowParams.amount);
    }
    // remove the revert
}

As a broad architectural recommendation, think about using a different way of storing failedMessages, the default way offered by LayerZero was intended for much simpler payloads.
The setup the protocol has is quite complex. Another recommendation is to not wrap a bunch of different logic in one big try/catch.

But try to split it into several steps so if let’s say the final withdrawal to another chain fails it does not revert the swapping and steps before that.

Consider using something like https://github.com/weiroll/weiroll which would chain multiple operations.

Other occurrences

• https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/modules/BaseTOFTLeverageModule.sol#L195-L200

• https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/modules/BaseTOFTMarketModule.sol#L170-L175

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

• https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/modules/BaseTOFTStrategyModule.sol#L163-L168

• https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/master/contracts/usd0/modules/USDOLeverageModule.sol#L180-L185

• https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/master/contracts/usd0/modules/USDOMarketModule.sol#L178-L186

• https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/master/contracts/usd0/modules/USDOOptionsModule.sol#L187-L197

Assessed type

Error

[c4-pre-sort]

minhquanym marked the issue as duplicate of #1410

[c4-judge]

dmvt marked the issue as satisfactory