c4-judge
commented
1 year ago Picodes marked the issue as unsatisfactory: Insufficient quality
Closed code423n4 closed 1 year ago
c4-judge
commented
1 year ago Picodes marked the issue as unsatisfactory: Insufficient quality
Lines of code
https://github.com/code-423n4/2023-06-stader/blob/main/contracts/StaderStakePoolsManager.sol#L215
Vulnerability details
The StaderStakePoolsManager contract contains a critical bug that could lead to financial loss and system instability. The bug is related to the handling of excess ETH deposits and the calculation of available ETH for new deposits.
Bug Description:
In the function depositETHOverTargetWeight(), the contract calculates the available ETH for new deposits as the difference between the contract's balance and the ETH requested for withdrawal by users. However, this calculation does not consider the amount of ETH already allocated for existing deposits in the pools. As a result, the contract may allow new deposits even when the available ETH is insufficient to cover the existing deposits.
The function depositETHOverTargetWeight() performs excess ETH allocation to pools based on selectedPoolCapacity, but it fails to update the excessETHDepositCoolDown variable. This means that the contract doesn't enforce the cooldown period between consecutive excess ETH deposits, potentially allowing excessive deposits within a short period.
In the function validatorBatchDeposit(), the contract subtracts the ETH requested for withdrawal from the contract's balance to determine the available ETH for new deposits. However, this calculation does not consider the ETH already allocated for existing deposits. As a result, the contract may allow new deposits even when the available ETH is insufficient to cover the requested deposit size for a particular pool.
Impact: These bugs could have severe consequences:
Loss of user funds: Users could deposit ETH into the contract, expecting it to be allocated to the pools correctly. However, due to the incorrect calculation of available ETH for new deposits, the contract may accept the deposits even if there are not enough funds available. This could result in the loss of user funds and damage to user trust. System instability: If the contract allows new deposits without sufficient available ETH, it could lead to underfunding of the pools. This could cause issues such as the inability to fulfill user withdrawal requests, inaccurate exchange rate calculations, and overall system instability. Recommendation: To address these critical bugs, the following changes are recommended:
In the depositETHOverTargetWeight() function, before allocating excess ETH to pools, ensure that the available ETH is sufficient to cover the existing deposits and the requested deposit sizes for the selected pools. If the available ETH is insufficient, revert the transaction and provide an appropriate error message.
Update the excessETHDepositCoolDown variable after each excess ETH deposit in the depositETHOverTargetWeight() function. This will enforce the cooldown period between consecutive excess ETH deposits, preventing excessive deposits within a short period.
In the validatorBatchDeposit() function, calculate the available ETH for new deposits by subtracting the existing deposits' ETH from the contract's balance. Consider the requested deposit size for the pool when checking if the available ETH is sufficient. If the available ETH is insufficient, revert the transaction and provide an appropriate error message.
Thoroughly test the updated contract to ensure the correct allocation of ETH to pools, accurate exchange rate calculations, and the overall stability of the system.
Assessed type
Access Control