Description

The finalizeValidatorRemoval function removes a validator by moving every validator after it one slot backwards in the validators array:

function finalizeValidatorRemoval() external onlyOwner {
    // [...]

    for (uint256 i = validatorId; i < validators.length - 1; ++i) {
        validators[i] = validators[i + 1];
        vaults[i] = vaults[i + 1];
    }

    validators.pop();
    vaults.pop();

    // [...]
}

This means that whenever finalizeValidatorRemoval is called by the owner at the same time as another call that references a validator by ID, and the referenced validator is after the validator that is being removed, then the block builder will have control over which validator is actually referenced by the other call.

Here is a list of functions that reference a vault or validator by ID:

• depositQueuedTokens(_vaultIds, _amounts)

• forceUnbond(_vaultIds, _amounts)

• unstakeClaim(_vaultIds)

• restakeRewards(_vaultIds)

• queueValidatorRemoval(_validatorId)

The depositQueuedTokens and forceUnbond functions are only callable by the deposit controller through the fund-flow controller, so that likely calculates the correct vault IDs using on-chain information.

The unstakeClaim and restakeRewards functions are permissionlessly callable, so messing with the vault IDs of someone else's call does not add to the threat surface aside from causing calls to unexpectedly revert.

The queueValidatorRemoval function is only callable by the owner after the finalizeValidatorRemoval is successfully called, so it cannot be reordered.

Impact

The only impact is that the block builder of a block that includes the finalizeValidatorRemoval call can cause some calls to unstakeClaim, restakeRewards, and queueValidatorRemoval to unexpectedly revert. This is a minor inconvenience.

Recommendations

We recommend refactoring the storage to not have to reassign IDs when a validator is removed. Iteration through the validators can still be done by maintaining a next-index pointer for each validator.

This will increase the friendliness of the external interface, because users and external code can store the ID as a permanent identifier of the particular vault.

Remediation