Finalized withdrawals can be modified

[howlbot-integration[bot]]

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/Withdraw/WithdrawQueue.sol#L206 https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/Withdraw/WithdrawQueue.sol#L279 https://github.com/code-423n4/2024-04-renzo/blob/1c7cc4e632564349b204b4b5e5f494c9b0bc631d/contracts/Withdraw/WithdrawQueue.sol#L129

Vulnerability details

Vulnerability Details:

The protocol implements a two-step process for withdrawals: first, the withdraw function in the WithdrawQueue contract is called by a user to create a withdrawal request. Once created, a user can call the claim function to claim the withdrawal request after the cooldown period has passed.

The issue with the current implementation is that the claim function uses the coolDownPeriod state variable to check if a user's withdrawal request has passed the period and met the condition.

    function claim(uint256 withdrawRequestIndex) external nonReentrant {
        // check if provided withdrawRequest Index is valid
        if (withdrawRequestIndex >= withdrawRequests[msg.sender].length) {
            revert InvalidWithdrawIndex();
        }

        WithdrawRequest memory _withdrawRequest = withdrawRequests[msg.sender][withdrawRequestIndex];

        if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim();
    ...
    }

This coolDownPeriod can be modified at any time by the WithdrawQueueAdmin, meaning that since the claim function uses the state variable, previous withdrawal requests that were finalized at the initial cooldown period will now have that time period changed and could have a longer wait.

    function updateCoolDownPeriod(uint256 _newCoolDownPeriod) external onlyWithdrawQueueAdmin {
        if (_newCoolDownPeriod == 0) revert InvalidZeroInput();
        emit CoolDownPeriodUpdated(coolDownPeriod, _newCoolDownPeriod);
        coolDownPeriod = _newCoolDownPeriod;
    }

Assume the following:

• User A calls the claim function to create a withdrawal request for funds needed before a specific date, and the cooldown period currently meets that date.
• The WithdrawQueueAdmin calls the updateCoolDownPeriod to increase the cooldown duration.
• User A now has to wait a longer period, even though their withdrawal request was finalized at the initial cooldown period.

Impact:

• Severity: Medium. The current implementation will lead to unexpected unfair delays for users who have already finalized their withdrawal requests.
• Likelihood: Low. The likelihood is dependent on the frequency of cooldown period changes

Tools Used:

• Manual analysis

Recommendation:

The protocol should calculate and add the release time to the specific withdrawal request. That way, any changes to the cooldown period will only affect future withdrawal requests.

    function withdraw(uint256 _amount, address _assetOut) external nonReentrant {
        ...
        uint256 releaseTime = block.timestamp + coolDownPeriod; //ADD HERE
        // add withdraw request for msg.sender
        withdrawRequests[msg.sender].push(
            WithdrawRequest(_assetOut, withdrawRequestNonce, amountToRedeem, _amount, block.timestamp, releaseTime) //ADD HERE
        );
        ...
    }

Assessed type

Other

[c4-judge]

alcueca marked the issue as duplicate of #607

[c4-judge]

alcueca marked the issue as unsatisfactory: Invalid

[c4-judge]

alcueca changed the severity to QA (Quality Assurance)

[c4-judge]

alcueca marked the issue as grade-b