MEDIUM severity: Unsafe ETH and Token Handling in the Bridge Contracts. The L1Mailbox contract inherits a receive() function from MailBoxBase, allowing direct ETH transfers to it, which can result in permanently locked funds as users may mistakenly send ETH instead of using the proper deposit function. Similarly, on L2, users can send native tokens directly to L2Mailbox...

Vulnerable code:
// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import "../interfaces/IL1Mailbox.sol";
import "../interfaces/IRollup.sol";
import "../../common/MailBoxBase.sol";
import "../libraries/verifier/WithdrawTrieVerifier.sol";
import "../interfaces/IL1MailQueue.sol";
import "@openzeppelin/contracts/utils/structs/DoubleEndedQueue.sol";

contract L1Mailbox is MailBoxBase, IL1Mailbox, IL1MailQueue {
    using DoubleEndedQueue for DoubleEndedQueue.Bytes32Deque;

    event SetL2GasLimit(uint256 oldGasLimit, uint256 newGasLimit);

    event SetL2FinalizeDepositGasUsed(uint256 oldL2FinalizeDepositGasUsed, uint256 newL2FinalizeDepositGasUsed);

    // double ended msg queue
    // begin is next finalize msg
    // end + 1 is next append msg
    DoubleEndedQueue.Bytes32Deque private msgQueue;

    bytes32 public stableRollingHash;

    // next pending msg index
    uint256 public pendingQueueIndex;

    // init with 0;
    uint256 public nextFinalizeQueueIndex;

    /// @notice The address of Rollup contract.
    address public rollup;

    /// @notice The gaslimit of L2, deposit gas limit must less than this value.
    uint256 public l2GasLimit;

    uint256 public feeBalance;

    address public withdrawer;

    uint256 public l2FinalizeDepositGasUsed;

    uint256 public lastestQueueIndex;

    modifier onlyRollup() {
        require(msg.sender == rollup, "Only callable by the Rollup");
        _;
    }

    modifier onlyWithdrawer() {
        require(msg.sender == withdrawer, "Only callable by the withdrawer");
        _;
    }

    constructor(){
        _disableInitializers();
    }

    /**
     * Contract initialization
     * @param rollup_ rollup contract address
     * @param owner_ contract owner address
     * @param baseFee_ base fee
     */
    function initialize(address rollup_, address owner_, uint256 baseFee_, uint256 _l2GasLimit, uint256 _l2FinalizeDepositGasUsed) external initializer {
        if (rollup_ == address(0) || owner_ == address(0)) {
            revert InvalidInitAddress();
        }

        if (_l2GasLimit < _l2FinalizeDepositGasUsed) {
            revert InvalidL2GasLimit();
        }
        __MailBox_init();

        rollup = rollup_;
        baseFee = baseFee_;
        l2GasLimit = _l2GasLimit;
        l2FinalizeDepositGasUsed = _l2FinalizeDepositGasUsed;
        _transferOwnership(owner_);
    }

    function setRollup(address rollup_) external whenPaused onlyOwner {
        require(rollup_ != address(0), "Invalid rollup address");
        rollup = rollup_;
    }

    function setWithdrawer(address _withdrawer) external onlyOwner {
        require(_withdrawer != address(0), "Invalid withdrawer address");
        withdrawer = _withdrawer;
    }

    function sendMsg(
        address target_,
        uint256 value_,
        bytes calldata msg_,
        uint256 gasLimit_,
        address refundAddress_
    ) external payable override onlyBridge whenNotPaused nonReentrant {
        // compute the actual cross domain message calldata.
        uint256 nonce_ = nextMsgIndex();
        bytes memory data_ = _encodeCall(_msgSender(), target_, value_, nonce_, msg_);

        // Calculate the fee and leave it in the MailBox contract
        uint256 fee_ = estimateMsgFee(gasLimit_);
        require(gasLimit_ < l2GasLimit, "gasLimit must less than L2 config");
        require(gasLimit_ >= l2FinalizeDepositGasUsed, "gas limit must be bigger than or equal to the tx_fee of finalize deposit on Jovay");
        require(msg.value >= fee_ + value_, "Insufficient msg.value");

        bytes32 hash_ = keccak256(data_);
        // normally this won't happen, since each message has different nonce, but just in case.
        _sendMsgCheck(hash_);

        // append message to L1MailQueue
        _appendMsg(_getRollingHash(hash_));

        emit SentMsg(_msgSender(), target_, value_, nonce_, data_, gasLimit_, hash_);

        // refund fee to `refundAddress_`
        unchecked {
            uint256 refund_ = msg.value - fee_ - value_;
            if (refund_ > 0) {
                (bool success_,) = refundAddress_.call{value : refund_}("");
                require(success_, "Failed to refund the fee");
            }
        }
        feeBalance += fee_;
    }

    /**
     * Send L2 message to L1, need to verify the validity of the message through proof,
     * if valid, execute the corresponding message in L1 bridge contract
     * @param sender_; sender L2 bridge contract address
     * @param target_; message receiver L1 bridge contract address
     * @param value_; native token transfer amount
     * @param nonce_; message nonce value
     * @param msg_; message content sent to target_ execution
     * @param proof_; proof information used to prove the validity of the message
     */
    function relayMsgWithProof(
        address sender_,
        address target_,
        uint256 value_,
        uint256 nonce_,
        bytes memory msg_,
        L2MsgProof memory proof_
    ) external payable whenNotPaused nonReentrant {
        require(sender_ == IBridge(target_).toBridge(), "Invalid sender");
        bytes32 hash_ = keccak256(_encodeCall(sender_, target_, value_, nonce_, msg_));

        bytes32 msgRoot_ = IRollup(rollup).getL2MsgRoot(proof_.batchIndex);
        require(
            WithdrawTrieVerifier.verifyMerkleProof(msgRoot_, hash_, nonce_, proof_.merkleProof),
            "Invalid proof"
        );

        (bool success,) = target_.call{value : value_}(msg_);
        require(success, "RelayMsg Failed");
        _receiveMsgCheck(hash_);
        emit RelayedMsg(hash_, nonce_);
    }

    function withdrawDepositFee(address _target, uint256 _amount) external onlyWithdrawer whenNotPaused {
        require(_target.code.length == 0, "INVALID_PARAMETER: withdraw target must be eoa");
        require(_amount <= feeBalance, "INVALID_PARAMETER : withdraw amount must smaller than or equal to fee in mailbox");
        feeBalance -= _amount;
        (bool success,) = _target.call{value : _amount}("");
        require(success, "INTERNAL_ERROR : withdraw fee Failed");
    }

    /**
     * @notice Set new L2 Gas limit for deposit
     */
    function setL2GasLimit(uint256 _l2GasLimit) external onlyOwner {
        if (l2FinalizeDepositGasUsed > _l2GasLimit) {
            revert SetL2GasLimitSmallerThanGasUsed();
        }
        uint256 oldL2GasLimit = l2GasLimit;
        l2GasLimit = _l2GasLimit;
        emit SetL2GasLimit(oldL2GasLimit, _l2GasLimit);
    }

    /**
     * @notice Set new L2 Gas used for finalize deposit
     */
    function setL2FinalizeDepositGasUsed(uint256 _l2FinalizeDepositGasUsed) external onlyOwner {
        if (_l2FinalizeDepositGasUsed > l2GasLimit) {
            revert SetL2FinalizeDepositGasUsedBiggerThanGasLimit();
        }
        uint256 oldL2FinalizeDepositGasUsed = l2FinalizeDepositGasUsed;
        l2FinalizeDepositGasUsed = _l2FinalizeDepositGasUsed;
        emit SetL2FinalizeDepositGasUsed(oldL2FinalizeDepositGasUsed, _l2FinalizeDepositGasUsed);
    }

    /**
     * @notice Returns next message index
     */
    function nextMsgIndex() public view override returns (uint256) {
        return pendingQueueIndex;
    }

    /**
     * @notice Returns message at index
     */
    function getMsg(uint256 _l1MsgCount) external view override returns (bytes32) {
        if (_l1MsgCount == 0) {
            return bytes32(0);
        }
        // totalIndex - 1 == index; index >= nextFinalizeQueueIndex or index = nextFinalizeQueueIndex - 1;
        require(_l1MsgCount >= lastestQueueIndex, "used msg must bigger than lastestQueueIndex");
        require(_l1MsgCount - 1 < pendingQueueIndex, "used msg must smaller than next pending");
        if (_l1MsgCount < lastestQueueIndex + 1) {
            return stableRollingHash;
        }
        return msgQueue.at(_l1MsgCount - lastestQueueIndex - 1);
    }

    /**
      * @notice set lastest queue index  called when pause
     */
    function setLastQueueIndex() external whenPaused onlyOwner {
        lastestQueueIndex = nextFinalizeQueueIndex;
    }

    /**
     * @notice Appends message to queue
     */
    function _appendMsg(bytes32 msg_) internal override {
        msgQueue.pushBack(msg_);
        pendingQueueIndex++;
        emit AppendMsg(pendingQueueIndex, msg_);
    }

    /**
     * @notice Pops messages from queue
     */
    function popMsgs(uint256 _l1MsgCount) external onlyRollup whenNotPaused {
        // l1MsgCount - 1 = index < pendingQueueIndex
        require(_l1MsgCount < pendingQueueIndex + 1, "finalize index must smaller than pendingQueueIndex");
        require(_l1MsgCount >= nextFinalizeQueueIndex, "finalize index must smaller than or equal to l1MsgCount");
        nextFinalizeQueueIndex = _l1MsgCount;
//        while (nextFinalizeQueueIndex < _l1MsgCount) {
//            stableRollingHash = msgQueue.popFront();
//            nextFinalizeQueueIndex++;
//        }
        emit PopMsgs(nextFinalizeQueueIndex);
    }

}

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import {AppendOnlyMerkleTree} from "../libraries/common/AppendOnlyMerkleTree.sol";
import "../interfaces/IL2Mailbox.sol";
import "../interfaces/IL2MailQueue.sol";
import "../../common/MailBoxBase.sol";

contract L2Mailbox is AppendOnlyMerkleTree, MailBoxBase, IL2Mailbox, IL2MailQueue {
    /// @notice The address of L1MailBox contract.
    address public l1MailBox;

    mapping(bytes32 => bool) public receiveMsgStatus;

    constructor(){
        _disableInitializers();
    }

    /**
     * Contract initialization
     * @param l1MailBox_ L1MailBox contract address
     * @param owner_ Contract owner address
     * @param baseFee_ Message sender token transfer amount
     */
    function initialize(address l1MailBox_, address owner_, uint256 baseFee_) external initializer {
        if (l1MailBox_ == address(0) || owner_ == address(0)) {
            revert InvalidInitAddress();
        }
        require(_nextMsgIndex == 0, "msg index is not 0");
        __MailBox_init();

        l1MailBox = l1MailBox_;
        baseFee = baseFee_;
        _transferOwnership(owner_);
        _initializeMerkleTree();
    }

    function setL1MailBox(address l1MailBox_) whenPaused external onlyOwner {
        require(l1MailBox_ != address(0), "Invalid address");
        l1MailBox = l1MailBox_;
    }

    function sendMsg(
        address target_,
        uint256 value_,
        bytes calldata msg_,
        uint256 gasLimit_,
        address refundAddress_
    ) external payable override onlyBridge whenNotPaused nonReentrant {

        // compute the actual cross domain message calldata.
        uint256 nonce_ = _nextMsgIndex;
        bytes memory data_ = _encodeCall(_msgSender(), target_, value_, nonce_, msg_);

        // Calculate the fee and keep it in the MailBox contract
        uint256 fee_ = estimateMsgFee(gasLimit_);
        require(msg.value >= fee_ + value_, "Insufficient msg.value");

        bytes32 hash_ = keccak256(data_);
        // normally this won't happen, since each message has different nonce, but just in case.
        _sendMsgCheck(hash_);
        // append message to L2MailQueue
        _appendMsg(hash_);

        emit SentMsg(_msgSender(), target_, value_, nonce_, msg_, gasLimit_, hash_);

        // refund fee to `refundAddress_`
        unchecked {
            uint256 refund_ = msg.value - fee_ - value_;
            if (refund_ > 0) {
                (bool success_,) = refundAddress_.call{value : refund_}("");
                require(success_, "Failed to refund the fee");
            }
        }
    }

    /**
     * Send L1 message to current L2 through Relayer
     * @param sender_ sender L1 bridge contract address
     * @param target_ message receiver current L2 bridge contract address
     * @param value_ native token transfer amount
     * @param nonce_ message queue nonce
     * @param msg_ message content sent to target_ execution
     */
    function relayMsg(
        address sender_,
        address target_,
        uint256 value_,
        uint256 nonce_,
        bytes calldata msg_
    ) external override whenNotPaused nonReentrant {
        // here l1MailBox will be set as L2Relayer 0x5100000000000000000000000000000000000000
        require(_msgSender() == l1MailBox, "Caller is not L1Mailbox");

        bytes32 hash_ = keccak256(_encodeCall(sender_, target_, value_, nonce_, msg_));
        bytes32 rollinghash = _getRollingHash(hash_);
        emit RollingHash(rollinghash);
        _receiveMsgCheck(hash_);
        (bool success,) = target_.call{value : value_}(msg_);
        if (success) {
            _receiveMsgSuccess(hash_);
            emit FinalizeDepositETHSuccess(hash_, nonce_);
        } else {
            _receiveMsgFailed(hash_);
            emit FinalizeDepositETHFailed(hash_, nonce_);
        }
        emit RelayedMsg(hash_, nonce_);
    }

    function claimAmount(
        address refundAddress_,
        uint256 amount_,
        uint256 nonce_,
        bytes32 msgHash_
    ) external override onlyBridge whenNotPaused nonReentrant {
        _checkMsgClaimValid(msgHash_);
        (bool success,) = refundAddress_.call{value : amount_}("");
        require(success, "claim amount failed when transfer to refund");
        _finalizeClaimMsg(msgHash_);

        emit ClaimMsg(msgHash_, nonce_);
    }

    /**
     * @dev Appends a message to the queue.
     */
    function _appendMsg(bytes32 msgHash) internal override {
        (uint256 currentNonce,) = _appendMsgHash(msgHash);
        // We can use the event to compute the merkle tree locally.
        emit AppendMsg(currentNonce, msgHash);
    }

    function msgRoot() external view returns (bytes32) {
        return _msgRoot;
    }

    function _receiveMsgFailed(bytes32 hash_) internal {
        receiveMsgStatus[hash_] = false;
    }

    function _receiveMsgSuccess(bytes32 hash_) internal {
        receiveMsgStatus[hash_] = true;
    }

    function _checkMsgClaimValid(bytes32 hash_) internal view {
        _msgExistCheck(hash_);
        require(!receiveMsgStatus[hash_], "ClaimMsg : L2 msg must exec failed before");
    }

    function _finalizeClaimMsg(bytes32 hash_) internal {
        _msgExistCheck(hash_);
        require(!receiveMsgStatus[hash_], "ClaimMsg : L2 msg must exec failed before");
        receiveMsgStatus[hash_] = true;
    }

}

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.0;

import {AddressUpgradeable} from "@openzeppelin/contracts-upgradeable/utils/AddressUpgradeable.sol";
import "@openzeppelin/contracts-upgradeable/token/ERC20/IERC20Upgradeable.sol";

import "./interfaces/IL1ETHBridge.sol";
import "../interfaces/IL1Mailbox.sol";
import "../../L2/bridge/interfaces/IL2ETHBridge.sol";
import "./L1BridgeProof.sol";

contract L1ETHBridge is L1BridgeProof, IL1ETHBridge {
    using AddressUpgradeable for address;
    uint256 public balance;

    function deposit(address to_, uint256 amount_, uint256 gasLimit_, bytes memory msg_) external payable override nonReentrant whenNotPaused {
        require(amount_ > 0, "deposit zero eth");
        // 1. Extract real sender if this call is from L1GatewayRouter.
        address sender_ = _msgSender();

        // 2. Generate message passed to L1Mailbox.
        bytes memory message_ = abi.encodeCall(IL2ETHBridge.finalizeDeposit, (sender_, to_, amount_, msg_));
        balance += amount_;
        mailBoxCall(abi.encodeCall(IMailBoxBase.sendMsg, (toBridge, amount_, message_, gasLimit_, sender_)));

        emit DepositETH(sender_, to_, amount_, msg_);
    }

    function finalizeWithdraw(address sender_, address to_, uint256 amount_, bytes memory msg_) external payable override nonReentrant onlyMailBox whenNotPaused {
        require(msg.value == amount_, "msg.value mismatch");
        require(balance >= amount_, "balance too low");

        // @note can possible trigger reentrant call to messenger,
        // but it seems not a big problem.
        balance -= amount_;
        (bool success_, ) = to_.call{value: amount_}("");
        require(success_, "ETH transfer failed");
// TODO : add call msg with withdraw
//        _doCallback(to_, msg_);

        emit FinalizeWithdrawETH(sender_, to_, amount_, msg_);
    }
}


Vulnerability type: CWE-707|CWE-20|CWE-1287