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

0 stars 0 forks source link

Fit Mango Moose - Excess fees are not sent back and stuck in SolConnector.sol #88

Open sherlock-admin4 opened 4 days ago

sherlock-admin4 commented 4 days ago

Fit Mango Moose

Medium

Excess fees are not sent back and stuck in SolConnector.sol

Summary

When the user wants to withdraw funds from vault on Solana, he has to pay the fees for cross-chain service to transfer the withdrawal request to Solana through the Layer Zero cross-chain infrastructure. In case of failed source message, or if after paying fees remained unused amount of native currency or lzToken, user expects the amount to be refunded. But current refunding mechanism is wrong and causes the fees to be stuck without the possibility of withdrawing them or returning them back to the user.

Root Cause

Link 1 In SolConnector.withdraw() refundAddress is specified as address(this), in this case address(this) is the SolConnector.sol, not the user that initiated the transaction. All excess fees Endpoint will send to SolConnector.sol. Moreover, in SolConnector.sol there is no function that can use the fees or extract them.

_lzSend(solEid, lzWithdrawMsg, withdrawOptions, _msgFee, address(this));

 function _lzSend(
        uint32 _dstEid,
        bytes memory _message,
        bytes memory _options,
        MessagingFee memory _fee,
        address _refundAddress   <<<
    ) internal virtual returns (MessagingReceipt memory receipt) {
       //...
            endpoint.send{ value: messageValue }(
                MessagingParams(_dstEid, _getPeerOrRevert(_dstEid), _message, _options, _fee.lzTokenFee > 0),
                _refundAddress   <<<
            );
    }

Link 2 Also we can see that msg.value not fully used - only _nativeFee will be sent to Endpoint. Remaining amount not sent back and stuck in SolConnector.sol.

uint256 messageValue = _payNative(_fee.nativeFee);
endpoint.send{ value: messageValue }(...);

 function _payNative(uint256 _nativeFee) internal virtual returns (uint256 nativeFee) {
        // enable the OApp to pay the native fee
        if (msg.value < _nativeFee && address(this).balance < _nativeFee) revert NotEnoughNative(msg.value);
        return _nativeFee;
    }

IMPORTANT NOTE

Sending msg.value greater than nativeFee is not a user mistake:

Because cross-chain gas fees are dynamic, this quote should be generated right before calling _lzSend to ensure accurate pricing. Sourсe

Users always need to overpay the fee to send message, thats why any protocols that implementing LayerZero must also implement the refunding mechanism correctly.

Internal pre-conditions

endpoint.quote() returns _fee.nativeFee that is less than msg.value.

External pre-conditions

Endpoint sends excess fees back to _refundAddress.

Attack Path

  1. Amelie makes a withdrawal request and sends msg.value = 0.2 ETH to pay for cross-chain gas fees.
  2. Only 0.15 ETH is used, Amelie expects the remaining 0.05 ETH to be sent to the refundAddress, back to her.
  3. 0.05 ETH is stuck in SolConnector.sol.

Impact

User lost all unused amount from msg.value and excess fees.

PoC

No response

Mitigation

Allow user to specify the _refundAddress and send whole msg.value to Endpoint, SolConnector.sol should not hold any excess fees.