sherlock-audit / 2024-02-tapioca-judging

2 stars 2 forks source link

cccz - For some cross-chain calls, the _toeComposeReceiver of BaseTOFTReceiver and USDOReceiver should check _srcChainSender == data.user #45

Closed sherlock-admin3 closed 7 months ago

sherlock-admin3 commented 7 months ago

cccz

high

For some cross-chain calls, the _toeComposeReceiver of BaseTOFTReceiver and USDOReceiver should check _srcChainSender == data.user

Summary

BaseTOFTReceiver and USDOReceiver's _toeComposeReceiver does not require _srcChainSender == data.user, which allows an attacker to steal the assets of an approved user

Vulnerability Detail

When a user initiates a cross-chain call, an arbitrary composeMsg can be constructed, which can use any address as data.user. In some functions, assets are transferred from data.user, e.g. when _msgType == MSG_DEPOSIT_LEND_AND_SEND_FOR_LOCK, in depositYBLendSGLLockXchainTOLP, assets are transferred from data.user and SGL is casted and given across the chain to the user.

        } else if (_msgType == MSG_DEPOSIT_LEND_AND_SEND_FOR_LOCK) {
            _executeModule(
                uint8(IUsdo.Module.UsdoMarketReceiver),
                abi.encodeWithSelector(
                    UsdoMarketReceiverModule.depositLendAndSendForLockingReceiver.selector, _toeComposeMsg
                ),
                false
            );
...
    function _depositYBLendSGL(
        IDepositData memory depositData,
        address singularityAddress,
        IYieldBox yieldBox_,
        address user,
        uint256 lendAmount
    ) internal returns (uint256 fraction) {
        if (singularityAddress != address(0)) {
            if (!cluster.isWhitelisted(0, singularityAddress)) {
                revert Magnetar_TargetNotWhitelisted(singularityAddress);
            }
            _setApprovalForYieldBox(singularityAddress, yieldBox_);

            IMarket singularity_ = IMarket(singularityAddress);

            // if `depositData.deposit`:
            //      - deposit SGL asset to YB for `user`
            uint256 sglAssetId = singularity_.assetId();
            (, address sglAssetAddress,,) = yieldBox_.assets(sglAssetId);
            if (depositData.deposit) {
                depositData.amount = _extractTokens(user, sglAssetAddress, depositData.amount);

Impact

This allows an attacker to cross-chain call depositYBLendSGLLockXchainTOLP using the approved user's address as data.user to steal the approved user's assets.

Code Snippet

https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/usdo/modules/UsdoReceiver.sol#L88-L95 https://github.com/sherlock-audit/2024-02-tapioca/blob/main/Tapioca-bar/contracts/usdo/modules/UsdoMarketReceiverModule.sol#L68-L93 https://github.com/Tapioca-DAO/tapioca-periph/blob/2ddbcb1cde03b548e13421b2dba66435d2ac8eb5/contracts/Magnetar/modules/MagnetarAssetXChainModule.sol#L70-L81 https://github.com/Tapioca-DAO/tapioca-periph/blob/2ddbcb1cde03b548e13421b2dba66435d2ac8eb5/contracts/Magnetar/modules/MagnetarAssetCommonModule.sol#L65-L85

Tool used

Manual Review

Recommendation

It is recommended to check _srcChainSender == data.user in the _toeComposeReceiver of BaseTOFTReceiver and USDOReceiver.

Duplicate of #14