Description

The function batchRevertExitRequest does not check if the _validatorIds passed to it are repeated nor does it check if an exit request has been previously requested for this validator ID. A malicious TNFT holder could use the same _validatorIds twice in an array, which would call IEtherFiNode(etherfiNode).updateNumExitRequests(0, 1); twice and would decrease the value of numExitRequestsByTnft by an unexpected amount. Or they could call batchRevertExitRequest with _validatorIds for which the exit request has not been previously requested, which would also decrease the value of numExitRequestsByTnft.

function batchRevertExitRequest(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");

        _updateEtherFiNode(_validatorId);

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

        emit NodeExitRequestReverted(_validatorId);
    }
}

Impact

An attacker could call this numerous times to decrease the value of numExitRequestsByTnft to 0. Later, if a validator ID is unregistered, the call would revert due to an underflow in the following line if the validator is exited using an exit request:

if (_info.exitRequestTimestamp != 0) numExitRequestsByTnft -= 1;

Recommendations

We recommend adding checks to ensure batchRevertExitRequest could only be called on a validator ID if an exit request has been previously requested for that ID.

Remediation

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