sherlock-audit / 2024-09-orderly-network-solana-contract-judging

0 stars 0 forks source link

Clumsy Powder Rook - The lack of payable modifier makes the withdraw function unusable #165

Open sherlock-admin2 opened 4 days ago

sherlock-admin2 commented 4 days ago

Clumsy Powder Rook

High

The lack of payable modifier makes the withdraw function unusable

Summary

Since the withdraw function lacks the payable modifier, the _lzSend function is called back inside the function, and the _lzSend function also lacks the payable modifier. In the _lzSend function, ETH transfer will be performed, so the transaction will fail.

Root Cause

    function _lzSend(
        uint32 _dstEid,
        bytes memory _message,
        bytes memory _options,
        MessagingFee memory _fee,
        address _refundAddress
    ) internal virtual returns (MessagingReceipt memory receipt) {
        // @dev Push corresponding fees to the endpoint, any excess is sent back to the _refundAddress from the endpoint.
        uint256 messageValue = _payNative(_fee.nativeFee);
        if (_fee.lzTokenFee > 0) _payLzToken(_fee.lzTokenFee);

        return
            // solhint-disable-next-line check-send-result
            endpoint.send{ value: messageValue }(
                MessagingParams(_dstEid, _getPeerOrRevert(_dstEid), _message, _options, _fee.lzTokenFee > 0),
                _refundAddress
            );
    }

The _lzSend function lacks the payable modifier, and ETH transfer will be performed in the _lzSend function, so the transaction will fail.

Internal pre-conditions

No response

External pre-conditions

No response

Attack Path

No response

Impact

withdraw function cannot be used

PoC

No response

Mitigation

function withdraw(WithdrawDataSol calldata _withdrawData) external onlyLedger 
+ payable
{
        AccountWithdrawSol memory withdrawData = AccountWithdrawSol(
            Utils.getSolAccountId(_withdrawData.sender, _withdrawData.brokerId),
            _withdrawData.sender,
            _withdrawData.receiver,
            Utils.calculateStringHash(_withdrawData.brokerId),
            Utils.calculateStringHash(_withdrawData.tokenSymbol),
            _withdrawData.tokenAmount,
            _withdrawData.fee,
            _withdrawData.chainId,
            _withdrawData.withdrawNonce
        );

        bytes memory payload = MsgCodec.encodeWithdrawPayload(withdrawData);
        bytes memory lzWithdrawMsg = MsgCodec.encodeLzMsg(uint8(MsgCodec.MsgType.Withdraw), payload);
        bytes memory withdrawOptions = OptionsBuilder.newOptions().addExecutorLzReceiveOption(
            msgOptions[uint8(MsgCodec.MsgType.Withdraw)].gas,
            msgOptions[uint8(MsgCodec.MsgType.Withdraw)].value
        );
        MessagingFee memory _msgFee = _quote(solEid, lzWithdrawMsg, withdrawOptions, false);
        _lzSend(solEid, lzWithdrawMsg, withdrawOptions, _msgFee, address(this));
    }