code-423n4 / 2024-02-tapioca-findings

1 stars 1 forks source link

`_lzCustomWithdraw` function of MagnetarBaseModule will not send any tokens cross-chain #155

Closed c4-bot-4 closed 5 months ago

c4-bot-4 commented 6 months ago

Lines of code

https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/Magnetar/modules/MagnetarBaseModule.sol#L131-L171

Vulnerability details

Description

The _withdrawToChain function of MagnetarBaseModule is used to withdraw tokens from YieldBox shares existing in the contract and send them cross-chain to the user's recipient. This function has an unwrap option to unwrap tokens after sending cross-chain, using the _lzCustomWithdraw function.

However, the _lzCustomWithdraw function only executes the lz calldata with the token amount to send being zero.

function _lzCustomWithdraw(
    address _asset,
    LZSendParam memory _lzSendParam,
    uint128 _lzSendGas,
    uint128 _lzSendVal,
    uint128 _lzComposeGas,
    uint128 _lzComposeVal,
    uint16 _lzComposeMsgType
) private {
    PrepareLzCallReturn memory prepareLzCallReturn = _prepareLzSend(_asset, _lzSendParam, _lzSendGas, _lzSendVal);

    TapiocaOmnichainEngineHelper _toeHelper = new TapiocaOmnichainEngineHelper();
    PrepareLzCallReturn memory prepareLzCallReturn2 = _toeHelper.prepareLzCall(
        ITapiocaOmnichainEngine(_asset),
        PrepareLzCallData({
            dstEid: _lzSendParam.sendParam.dstEid,
            recipient: _lzSendParam.sendParam.to,
            amountToSendLD: 0,
            minAmountToCreditLD: 0,
            msgType: _lzComposeMsgType,
            composeMsgData: ComposeMsgData({
                index: 0,
                gas: _lzComposeGas,
                value: prepareLzCallReturn.msgFee.nativeFee.toUint128(),
                data: _lzSendParam.sendParam.composeMsg,
                prevData: bytes(""),
                prevOptionsData: bytes("")
            }),
            lzReceiveGas: _lzSendGas + _lzComposeGas,
            lzReceiveValue: _lzComposeVal
        })
    );

    if (msg.value < prepareLzCallReturn2.msgFee.nativeFee) {
        revert Magnetar_GasMismatch(prepareLzCallReturn2.msgFee.nativeFee, msg.value);
    }

    IOftSender(_asset).sendPacket{value: prepareLzCallReturn2.msgFee.nativeFee}(
        prepareLzCallReturn2.lzSendParam, prepareLzCallReturn2.composeMsg
    );
}

As you can see in the above code snippet, prepareLzCallReturn was prepared as calldata to send tokens without composing a message but has not been executed. It only executes prepareLzCallReturn2, which represents the lz calldata for a composed message. It is used for custom options after receiving tokens. However, the amountToSendLD of prepareLzCallReturn2 is 0, resulting in this call not sending any tokens cross-chain.

Impact

Users will lose funds when they use a Magnetar function which triggers _lzCustomWithdraw. For example, using withdrawToChain() of MagnetarYieldBoxModule with the unwrap option (data.unwrap == true).

Tools Used

Manual review

Recommended Mitigation Steps

Should pass variable from data to amountToSendLD and minAmountToCreditLD to prepare calldata prepareLzCallReturn2

Assessed type

Other

c4-sponsor commented 6 months ago

cryptotechmaker (sponsor) disputed

cryptotechmaker commented 6 months ago

Invalid.

Tokens are sent by PrepareLzCallReturn memory prepareLzCallReturn = _prepareLzSend(_asset, _lzSendParam, _lzSendGas, _lzSendVal);

trungore commented 6 months ago

Hi @cryptotechmaker, from what I can observe in the code here: https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/Magnetar/modules/MagnetarBaseModule.sol#L131-L199 the prepareLzCallReturn function is crafted, but it's not utilized to send the packet. Only the prepareLzCallReturn2 is dispatched.

Also I think the #62 and #59 are not the duplications of this issue as well since they refer to a different problem.

c4-judge commented 6 months ago

dmvt marked the issue as unsatisfactory: Invalid

c4-judge commented 6 months ago

dmvt marked the issue as unsatisfactory: Invalid

c4-judge commented 6 months ago

dmvt marked the issue as unsatisfactory: Invalid

huuducsc commented 5 months ago

@dmvt I believe this issue is valid because in the _lzCustomWithdraw function, prepareLzCallReturn is crafted but it's not utilized to send the packet. Only prepareLzCallReturn2 is used to send the packet, which doesn't send any tokens. Therefore, tokens will be stuck as this function will not send tokens cross-chain. You can see the code snippet above to confirm.

c4-judge commented 5 months ago

dmvt removed the grade

c4-judge commented 5 months ago

dmvt marked issue #62 as primary and marked this issue as a duplicate of 62

dmvt commented 5 months ago

Hi @cryptotechmaker, from what I can observe in the code here: https://github.com/Tapioca-DAO/tapioca-periph/blob/032396f701be935b04a7e5cf3cb40a0136259dbc/contracts/Magnetar/modules/MagnetarBaseModule.sol#L131-L199 the prepareLzCallReturn function is crafted, but it's not utilized to send the packet. Only the prepareLzCallReturn2 is dispatched.

Also I think the #62 and #59 are not the duplications of this issue as well since they refer to a different problem.

59 is indeed a different issue. This one is now grouped with #62 which describes the core issue of the cross chain message failing to send. #62 does a much cleaner job of describing the issue and fixing it would also resolve this issue.

c4-judge commented 5 months ago

dmvt changed the severity to 2 (Med Risk)

c4-judge commented 5 months ago

dmvt marked the issue as satisfactory