Liquid Staking
Stakelink
DeFiHardhatOracle
50,000 USDC
View results
Previous Next
Submission Details
Severity: medium
Invalid

OperatorStakingPool::removeOperators() will revert due to index out of bounds error

ravikiranweb3

Summary

The removeOperatorsfunction accepts an array of operators to be removed from the operators array. But the problem arises due to issue in the logic where element of the array is being popped off while the loop iterates over the initial length.

As a result, calling removeOperators function will throw Index out of bounds.

Vulnerability Details

Refer to the below code where the length of the operators is stored in a variable named numOperators

Due to the structure of the logic, note how numOperators is the original length of the operators array.

As the remove operators are removed from the array, the length of the operators array will reduce in length, but the logic attempts to iterate the numOperatorswhich is the original length before removing any operators.

Hence, as operators are being removed during the loop, for the second element, the Index out of bounds error will be thrown.

function removeOperators(address[] calldata _operators) external onlyOwner {
uint256 numOperators = operators.length;
​
for (uint256 i = 0; i < _operators.length; ++i) {
address operator = _operators[i];
if (!isOperator(operator)) revert OperatorNotFound();
​
uint256 staked = getOperatorStaked(operator);
if (staked != 0) {
_withdraw(operator, staked);
}
​
operatorMap[operator] = false;
for (uint256 j = 0; j < numOperators; ++j) {
if (operators[j] == operator) {
operators[j] = operators[numOperators - 1];
operators.pop();
--numOperators;
}
}
}
}

Example: operators = [0,1,2,3,4,5,6]

numOperators = 7

_operators = [2,3,5]

As 2 is pop out of the operators array, array index at 6 will throw index out of bounds array as the length of array is now 6.

Impact

Operators cannot be removed by the owner

Tools Used

Manual

Recommendations

In order to fix this issue, initialize the numOperators variable inside the for loop.

function removeOperators(address[] calldata _operators) external onlyOwner {
for (uint256 i = 0; i < _operators.length; ++i) {
address operator = _operators[i];
if (!isOperator(operator)) revert OperatorNotFound();
​
uint256 staked = getOperatorStaked(operator);
if (staked != 0) {
_withdraw(operator, staked);
}
==> uint256 numOperators = operators.length;
operatorMap[operator] = false;
for (uint256 j = 0; j < numOperators; ++j) {
if (operators[j] == operator) {
operators[j] = operators[numOperators - 1];
operators.pop();
--numOperators;
}
}
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[INVALID] OperatorStakingPool::removeOperators() will revert due to index out of bounds error

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.