Adding Block.number to Block.timestamp

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/StaderStakePoolsManager.sol#L57 https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/StaderStakePoolsManager.sol#L215-L218

Vulnerability details

Impact

Detailed description of the impact of this finding. In the highlighted functions, the lastExcessETHDepositBlock is updated every time the function depositETHOverTargetWeight is called because it is set to the last block.number. If the function is called again, the lastExcessETHDepositBlock is checked with a cool down period against the current block.number, this introduces a critical flaw where the block.number value is added to a timestamp value resulting in a large value which is checked against the current block.number. This causes a DOS in the contract because the additions and checks are done based on the uint values as you cannot directly check the amount of seconds that has passed since a block by simply adding it to a block number.

Proof of Concept

Provide direct links to all referenced code in GitHub. Add screenshots, logs, or any other relevant proof that illustrates the concept.

function depositETHOverTargetWeight() external override nonReentrant {
        if (block.number < lastExcessETHDepositBlock + excessETHDepositCoolDown) {
            revert CooldownNotComplete();
        }
}

Tools Used

Manual review

Recommended Mitigation Steps

The excessETHDepositCoolDown should be set to a required amount of blocks not a required amount of seconds past.

Assessed type

DoS

[Picodes]

excessETHDepositCoolDown is an amount of block?

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid