Description

The function batchSendExitRequest does not check if the _validatorIds passed to it are repeated. A malicious TNFT holder could use the same _validatorIds twice in an array, which would call IEtherFiNode(etherfiNode).updateNumExitRequests(1, 0); twice and would increase the value of numExitRequestsByTnft by an unexpected amount.

function batchSendExitRequest(uint256[] calldata _validatorIds) external whenNotPaused {
    for (uint256 i = 0; i < _validatorIds.length; i++) {
        uint256 _validatorId = _validatorIds[i];
        address etherfiNode = etherfiNodeAddress[_validatorId];

        require (msg.sender == tnft.ownerOf(_validatorId), "NOT_TNFT_OWNER");
        require (phase(_validatorId) == IEtherFiNode.VALIDATOR_PHASE.LIVE, "NOT_LIVE");

        _updateEtherFiNode(_validatorId);

        IEtherFiNode(etherfiNode).updateNumExitRequests(1, 0);
        validatorInfos[_validatorId].exitRequestTimestamp = uint32(block.timestamp);

        emit NodeExitRequested(_validatorId);
    }
}

Impact

The attacker can increase the number of exit requests by repeating the same _validatorId. The function _getTotalRewardsPayoutsFromSafe, which is called to calculate the total rewards payout would revert if numExitRequestsByTnft is nonzero. An attacker could thus increase the value of numExitRequestsByTnft by using repeated validator IDs, which would make it impossible to claim the staking rewards.

Recommendations

We recommend reverting the function if there are repeated validator IDs.

Remediation

This issue has been acknowledged by EtherFi, and a fix was implemented in commit a15c35fd↗.