Fit Mango Moose - Malicious user user can aviod paying LZ fees and steal other users excess fees

[sherlock-admin2]

Fit Mango Moose

Medium

Malicious user user can aviod paying LZ fees and steal other users excess fees

Summary

Due to wrong check in OAppSenderUpgradeable._payNative(), malicious user can aviod paying LZ fees and steal other users excess fees - he can send msg.value = 0, but the transaction will not revert, because the excess fees of other users accumulated on the contract balance will be used to pay for the cross-chain message.

Root Cause

Link This line should check if the user has sent enough funds to pay cross-chain gas fee:

if (msg.value < _nativeFee && address(this).balance < _nativeFee) revert NotEnoughNative(msg.value);

But a second check is also performed - whether the balance of the contract is greater than the fees, and these two checks are connected by a logical AND. This if block will revert only if both conditions are true. But if msg.value = 0, and address(this).balance >= _nativeFee, meaning true and false, then contitions for revert will not meet.

Next, lets see at _payNative() function - amount that will be sent to Endpoint does not depend on msg.value, meaning that message can be executed even if user sends 0 amount of fees:

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;
    }

Balance of address(this) can be greater than nativeFee, because the contract holds excess fees, that was not refunded to users.

Internal pre-conditions

Balance of address(this) needs to be greater than nativeFee. It's possible because the contract holds excess fees, that was not refunded to users. Users always need to overpay the fee to send message, because cross-chain gas fees are dynamic. But due to some reasons, excess fees refunded to address(this), and malicious user can steal them.

External pre-conditions

None.

Attack Path

1. Other users make cross-chain withdrawals, and excess fees are accumulated on the contract balance, address(this).balance = 1000.
2. Amelie observes until the contract balance is sufficient to cover LZ fees, _nativeFee = 900.
3. Amelie makes a withdrawal request and sends msg.value = 0, i.e. does not pay any fees.
4. (msg.value < _nativeFee && address(this).balance < _nativeFee) = (0 < 900 && 1000 < 900) = true && false.
5. Withdrawal request is successful because there was no revert in _payNative() and the excess fees belonging to other users were used for paying cross-chain gas fees.

Impact

Other users lost excess fees belonging to them, malicious user avoided paying LayerZero fees and lost nothing.

PoC

See Attack Path.

Mitigation

Always make sure that msg.value >= _nativeFee:

 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);
+        if (msg.value < _nativeFee || address(this).balance < _nativeFee) revert NotEnoughNative(msg.value);
        return _nativeFee;
    }