Ineffective Pausing Mechanism

[c4-bot-9]

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/Bridge/L2/xRenzoDeposit.sol#L168
https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/Bridge/L2/xRenzoDeposit.sol#L204

Vulnerability details

Vulnerability Details:

The protocol has the ability to pause user deposits and withdrawals if necessary, for instance, due to the identification of a vulnerability or during a contract upgrade. However, the current implementation’s pausing capability does not account for deposits made from Layer 2 networks. As seen below, the depositETH and deposit functions in the xRenzoDeposit L2 contract lack a clear way to be halted if necessary.

    function depositETH(uint256 _minOut, uint256 _deadline) external payable nonReentrant returns (uint256) {
        ...
        return _deposit(wrappedAmount, _minOut, _deadline);
    }

    function deposit(uint256 _amountIn, uint256 _minOut, uint256 _deadline) external nonReentrant returns (uint256) {
        ...
        return _deposit(_amountIn, _minOut, _deadline);
    }

If the protocol needs to halt deposits and withdrawals for any reason, the current implementation cannot apply this restriction to deposits from any L2 networks. As a result, deposits can still be made while the protocol is paused, eventually reaching the L1 side and undermining the effectiveness of the pause

Impact:

• Severity: High. Since L2 deposits eventually flow into L1 and are part of the same pool, there might be cases where the protocol needs to pause all deposits. However, in the current implementation, this is not possible, which could lead to further issues if pausing becomes necessary.
• Likelihood: Low. This problem only occurs if the system is paused. However, since the functionality is implemented, it is reasonable to assume the possibility of its occurrence.

Tools Used:

• Manual analysis

Recommendation:

The protocol should add the pausing functionality to the depositETH and deposit functions in the xRenzoDeposit contract. This will ensure that the protocol can pause all deposits and withdrawals if necessary.

    /// @dev Only allows execution if contract is not paused
    modifier notPaused() {
        if (paused) revert ContractPaused();
        _;
    }
    
    ...
    
    function depositETH(uint256 _minOut, uint256 _deadline) external payable nonReentrant notPaused returns (uint256) {
        ...
        return _deposit(wrappedAmount, _minOut, _deadline);
    }
    
    ...
    
    function deposit(uint256 _amountIn, uint256 _minOut, uint256 _deadline) external nonReentrant notPaused returns (uint256) {
        ...
        return _deposit(_amountIn, _minOut, _deadline);
    }

Assessed type

Access Control

[jatinj615]

L2 deposits can be halted by removing the ability of xRenzoDeposit to mint xezETH instead.

[alcueca]

The mitigation by the sponsor is reasonable, although pausing might be more comfortable in the event of an incident. Valid QA.

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-a