sherlock-audit / 2024-02-rio-network-core-protocol-judging

4 stars 4 forks source link

hash - Adding multiple validators within security review period will increase the confirmation timestamp for all #356

Closed sherlock-admin4 closed 8 months ago

sherlock-admin4 commented 8 months ago

hash

medium

Adding multiple validators within security review period will increase the confirmation timestamp for all

Summary

Adding multiple validators within security review period will increase the confirmation timestamp for all

Vulnerability Detail

Validators are only usable for deposits once a security review period has passed without the validator being flagged and removed. This time is recorded using a confirmationTimestamp which is global to all the validators of an operator. Hence if an operator adds multiple validators within the same review period, the entire set of validators including ones that were added before will now have the confirmation timestamp of the latest added validator.

    function addValidatorDetails(
        uint8 operatorId,
        uint256 validatorCount,
        bytes calldata publicKeys,
        bytes calldata signatures
    ) external onlyOperatorManager(operatorId) {
        OperatorDetails storage operator = s.operatorDetails[operatorId];
        OperatorValidatorDetails memory validators = operator.validatorDetails;

        if (validatorCount == 0) revert INVALID_VALIDATOR_COUNT();

        // First check if there are any pending validator details that can be moved into a confirmed state.
        if (validators.total > validators.confirmed && block.timestamp >= validators.nextConfirmationTimestamp) {
            operator.validatorDetails.confirmed = validators.confirmed = validators.total;
        }

        operator.validatorDetails.total = VALIDATOR_DETAILS_POSITION.saveValidatorDetails(
            operatorId, validators.total, validatorCount, publicKeys, signatures
        );
=>      operator.validatorDetails.nextConfirmationTimestamp = uint40(block.timestamp + s.validatorKeyReviewPeriod);

Impact

Validators can get delayed for deposits when multiple validators are added by same operator within the same confirmation period

Code Snippet

https://github.com/sherlock-audit/2024-02-rio-network-core-protocol/blob/4f01e065c1ed346875cf5b05d2b43e0bcdb4c849/rio-sherlock-audit/contracts/restaking/RioLRTOperatorRegistry.sol#L261-L280

Tool used

Manual Review

Recommendation

Can avoid this by adding confirmation timestamp batchwise but it might be much better to just inform the operators of this nuance

Duplicate of #171