Description/Impact:
* @notice Permanently disables an operator
* @dev used when an operator misbehaves
* @param _operatorId id of operator
*/
function disableOperator(uint256 _operatorId) external onlyOwner operatorExists(_operatorId) {
require(operators[_operatorId].active, "Operator is already disabled");
uint256 unusedKeys = operators[_operatorId].validatorLimit -
operators[_operatorId].usedKeyPairs;
if (unusedKeys > 0) {
queueLength -= unusedKeys;
operators[_operatorId].validatorLimit -= uint64(unusedKeys);
currentStateHash = keccak256(
abi.encodePacked(currentStateHash, "disableOperator", _operatorId)
);
}
uint256 unstoppedValidators = operators[_operatorId].usedKeyPairs -
operators[_operatorId].stoppedValidators;
if (unstoppedValidators > 0) {
activeValidators[operators[_operatorId].owner] -= unstoppedValidators;
totalActiveValidators -= unstoppedValidators;
}
operators[_operatorId].active = false;
}
i. In the function disableOperator:: OperatorController
, the variable unstoppedValidators
should be initialized as int256 as there is a check if (unstoppedValidators)
. If unstoppedValidators
is assigned uint256 it can never be negative making the if statement not to be effective
ii. Also, if operators[_operatorId].usedKeyPairs < operators[_operatorId].stoppedValidators
the transaction will be reverted thereby causing onlyOwner not to be able to disable an operator who misbehaves.
Recommended Mitigation:
i. Initializing the unstoppedValidators
as int256 would make the if statement
effective and also avoid revert due to underflow if operators[_operatorId].usedKeyPairs < operators[_operatorId].stoppedValidators
ii. The variables operators[_operatorId].usedKeyPairs , operators[_operatorId].stoppedValidators
should rounded up as int64 also
* @notice Permanently disables an operator
* @dev used when an operator misbehaves
* @param _operatorId id of operator
*/
function disableOperator(uint256 _operatorId) external onlyOwner operatorExists(_operatorId) {
require(operators[_operatorId].active, "Operator is already disabled");
- uint256 unusedKeys = operators[_operatorId].validatorLimit - operators[_operatorId].usedKeyPairs;
+ int256 unusedKeys = int64(operators[_operatorId].validatorLimit)-int64(operators[_operatorId].usedKeyPairs);
if (unusedKeys > 0) {
queueLength -= unusedKeys;
operators[_operatorId].validatorLimit -= uint64(unusedKeys);
currentStateHash = keccak256(
abi.encodePacked(currentStateHash, "disableOperator", _operatorId)
);
}
-uint256 unstoppedValidators = operators[_operatorId].usedKeyPairs - operators[_operatorId].stoppedValidators;
+int256 unstoppedValidators = int64(operators[_operatorId].usedKeyPairs) - int64(operators[_operatorId].stoppedValidators);
if (unstoppedValidators > 0) {
activeValidators[operators[_operatorId].owner] -= unstoppedValidators;
totalActiveValidators -= unstoppedValidators;
}
operators[_operatorId].active = false;
}