code-423n4 / 2024-04-renzo-validation

2 stars 2 forks source link

Inadequate validation of `_coolDownPeriod` during Initialization allow for early withdrawal claims #207

Closed c4-bot-3 closed 5 months ago

c4-bot-3 commented 5 months ago

Lines of code

https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Withdraw/WithdrawQueue.sol#L78 https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Withdraw/WithdrawQueue.sol#L287

Vulnerability details

Summary

According to Documentation:

Depending on the restaking strategies deployed, unstaking will take a minimum of 7 days, primarily due to EigenLayer unstaking requirements, but will vary depending on each AVS.

Vulnerability stems from the current implementation of the initialize() function which does not enforce the documented requirement that the unstaking or claim cooldown period (coolDownPeriod) must be at least 7 days.

Proof of Concept

The initialization check for _coolDownPeriod only ensures that it is not set to zero.

initialize()


function initialize(
// ... other parameters ...
uint256 _coolDownPeriod,
// ... other parameters ...
) external initializer {
// ... other initialization code ...
// @audit This check is insufficient as it only prevents a cooldown period of 0
if (_coolDownPeriod == 0) revert InvalidZeroInput();

coolDownPeriod = _coolDownPeriod;

// ... rest of the initialization code ...

}

The insufficient validation of `_coolDownPeriod` in the `initialize()` function can lead to a situation where the `coolDownPeriod` is set to less than `7 days`.

If a `coolDownPeriod` shorter than `7 days` is set in `initialize()`, and [updateCoolDownPeriod()](https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Withdraw/WithdrawQueue.sol#L129-L133) is used later to extend it, there's a window where users could initiate a `withdraw()` and then `claim()` their funds before the cooldown is properly extended.

###### Exploit scenario
1. The contract is initialized with a `_coolDownPeriod` of less than `7 days`.
2. A user calls [withdraw()](https://github.com/code-423n4/2024-04-renzo/blob/519e518f2d8dec9acf6482b84a181e403070d22d/contracts/Withdraw/WithdrawQueue.sol#L206) to initiate an unstaking request:
```solidity
function withdraw(uint256 _amount, address _assetOut) external nonReentrant {
    // ... code to handle withdrawal ...
}
  1. The user then waits for the cooldown period (which is less than 7 days) to elapse. After the cooldown, the user calls claim() to complete the unstaking.
Breakdown

In the claim() function, the coolDownPeriod is checked to ensure that the user can only claim their funds after the coolDownPeriod has elapsed since their _withdrawRequest.

function claim(uint256 withdrawRequestIndex) external nonReentrant {
    // ... other code ...

    WithdrawRequest memory _withdrawRequest = withdrawRequests[msg.sender][withdrawRequestIndex];
    if (block.timestamp - _withdrawRequest.createdAt < coolDownPeriod) revert EarlyClaim();

    // ... code to handle claim ...
}

The claim() function retrieves the user's _withdrawRequest and checks the current timestamp against the createdAt timestamp of the _withdrawRequest. If the difference is less than coolDownPeriod, the transaction is reverted with EarlyClaim(), preventing the user from claiming their funds too early.

However, if the coolDownPeriod was initially set to less than 7 days during the contract's initialization, users could claim their funds before the intended 7-day minimum, exploiting the insufficient validation.

Impact

Users can withdraw and claim funds earlier than the intended 7-day cooldown, violating the staking policy.

Tools Used

Manual Review

Recommended Mitigation Steps

Enforce a minimum 7-day cooldown period in the initialize() function.

function initialize(
    // other parameters
    uint256 _coolDownPeriod,
    // other parameters
) external initializer {
    // existing checks and code

    // @audit Enforce a minimum 7-day cooldown period
    if (_coolDownPeriod < 7 days) revert CoolDownPeriodTooShort();

    // @audit Set the coolDownPeriod
    coolDownPeriod = _coolDownPeriod;

    // rest of the initialization code
}

This additional check ensures that _coolDownPeriod is at least 7 days, and if not, it reverts.

Assessed type

Invalid Validation

0xJuancito commented 5 months ago

Overinflated severity. Also falls into the Known Risks "Incorrect configuration being set or admin privileged functions causing invalid configuration", given that an admin would have to set a different configuration than the referred in the docs.

0xJuancito commented 5 months ago

@howlbot reject