LayerZero OFT Arbitrum Native Contracts Security Review Report

The address variable bridge in contract OrbitERC20OFTAdapter is only set once during the constructor, and there is no setter function to modify its value. Therefore, we can declare bridge as an immutable variable to save gas.

IBridge private bridge;

Consider changing the bridge variable to immutable.

In the contract NativeOFTAdapterMsgValueTransfer, the function _payNative() is overridden to adapt to the new implementation of the function send(), which allows users to mint OFT tokens using the msg.value when the msgSenderBalance < _sendParam.amountLD. The nativeFee is then modified to msg.value - mintAmount to account for the mint amount.

However, when the msgSenderBalance > _sendParam.amount, there is no verification between the msg.value and the feeWithExtraAmount.nativeFee, other than the requirement that msg.value must be greater than feeWithExtraAmount.nativeFee in the _payNative() function.

This lack of verification will lead to a scenario where:

• msg.value > feeWithExtraAmount.nativeFee AND
• msgSenderBalance > _sendParam.amount This will result in the amount msg.value - feeWithExtraAmount.nativeFee of native tokens being lost in the contract.

function _payNative(uint256 _nativeFee) internal override returns (uint256 nativeFee) {
    if (msg.value < _nativeFee) revert NotEnoughNative(msg.value);
    return _nativeFee;
}

if (msgSenderBalance < _sendParam.amountLD) {
    require(msgSenderBalance + msg.value >= _sendParam.amountLD, "NativeOFTAdapterMsgValueTransfer: Insufficient msg.value");
 
    // user can cover difference with additional msg.value ie. wrapping
    uint mintAmount = _sendParam.amountLD - msgSenderBalance;
    _mint(address(msg.sender), mintAmount);
 
    // update the messageFee to take out mintAmount
    feeWithExtraAmount.nativeFee = msg.value - mintAmount;
}

Consider after line 128 adding a else block to verify whether msg.value == feeWithExtraAmount.nativeFee

  if (msgSenderBalance < _sendParam.amountLD) {
      require(msgSenderBalance + msg.value >= _sendParam.amountLD, "NativeOFTAdapterMsgValueTransfer: Insufficient msg.value");
 
      // user can cover difference with additional msg.value ie. wrapping
      uint mintAmount = _sendParam.amountLD - msgSenderBalance;
      _mint(address(msg.sender), mintAmount);
 
      // update the messageFee to take out mintAmount
      feeWithExtraAmount.nativeFee = msg.value - mintAmount;
  }
+ else {
+     require(msg.value == feeWithExtraAmount.nativeFee);
+ }

The NativeOFTAdapterMsgValueTransfer.sol contract's various errors are currently handled using native Solidity revert statements and custom errors, with some exceptions.

For example:

require(success, "NativeOFTAdapterMsgValueTransfer: failed to unwrap");

src/L3/NativeOFTAdapterMsgValueTransfer.sol:L85

require(success, "NativeOFTAdapterMsgValueTransfer: failed to unwrap");

src/L3/NativeOFTAdapterMsgValueTransfer.sol:L97

require(balanceOf(msg.sender) >= _amount, "NativeOFTAdapterMsgValueTransfer: Insufficient balance.");

src/L3/NativeOFTAdapterMsgValueTransfer.sol:L100

require(success, "NativeOFTAdapterMsgValueTransfer: failed to unwrap");

src/L3/NativeOFTAdapterMsgValueTransfer.sol:L120

require(msgSenderBalance + msg.value >= _sendParam.amountLD, "NativeOFTAdapterMsgValueTransfer: Insufficient msg.value");

Replace require statements with Custom Errors for a more streamlined and user-friendly experience. Furthermore, custom errors are much clearer as they allow for parameter values, making debugging much easier.

In the function NativeOFTAdapterMsgValueTransfer._debit(), the OFT tokens are transferred to the contract itself. Since the contract has no mechanism to manage these tokens, they are effectively rendered "dead".

_transfer(msg.sender, address(this), amountSentLD);

It is recommended to burn the OFT tokens from the sender instead of transferring them to the OFT contract.