The increaseTotalValidatorActiveCount in PermissionedPool incorrectly adds requiredValidators instead of validatorToDeposit

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/PermissionedPool.sol#L125

Vulnerability details

Impact

When the Stader Stake Pools Manager calls stakeUserETHToBeacon chain, it does so calculating the requiredValidators that can be added to the pool. The function internally also uses the allocateValidatorsAndUpdaterOperatorId to compute each operator's capacity. StakeUserETHToBeaconChain then will do a preDeposit on the beacon chain using the value calculated from allocateValidatorsAndUpdateOperators. This means that only validatorToDeposit validators are actually active, however the function will add requiredValidators to the PermissionedNodeRegistry's totalActiveValidatorCount. This will lead to incorrect calculations for any subsequent validator deposits.

Proof of Concept

https://github.com/code-423n4/2023-06-stader/blob/7566b5a35f32ebd55d3578b8bd05c038feb7d9cc/contracts/PermissionedPool.sol#L125

Tools Used

Manual review.

Recommended Mitigation Steps

Only add validatorToDeposit to the totalValidatorCount.

Assessed type

Loop

[c4-judge]

Picodes marked the issue as duplicate of #283

[c4-judge]

Picodes marked the issue as satisfactory

[c4-judge]

Picodes changed the severity to 2 (Med Risk)

[c4-judge]

Picodes marked the issue as unsatisfactory: Invalid