Summary
There is no limit for queueing lock updates in SDLPoolSecondary. As a result, a user can get his lock blocked forever if he queues too many updates in the same indexBatch
Vulnerability Details
In SDLPoolSecondary we can see that there is a limit for queuing new locks per user.
function _queueNewLock(
address _owner,
uint256 _amount,
uint64 _lockingDuration
) internal {
if (newLocksByOwner[_owner].length >= queuedNewLockLimit) revert TooManyQueuedLocks(); <-
Lock memory lock = _createLock(_amount, _lockingDuration);
queuedNewLocks[updateBatchIndex].push(lock);
newLocksByOwner[_owner].push(NewLockPointer(updateBatchIndex, uint128(queuedNewLocks[updateBatchIndex].length - 1)));
queuedRESDLSupplyChange += int256(lock.amount + lock.boostAmount);
if (updateNeeded == 0) updateNeeded = 1;
emit QueueCreateLock(_owner, _amount, lock.boostAmount, _lockingDuration);
}
However, there is no limit for queueing lock updates. That means that a user unconsiously can queue too many lock updates in the same index batch and get his lock blocked.
When queueing lock updates we can see that there is no check for a limit
function _queueLockUpdate(
address _owner,
uint256 _lockId,
uint256 _amount,
uint64 _lockingDuration
) internal onlyLockOwner(_lockId, _owner) {
Lock memory lock = _getQueuedLockState(_lockId);
LockUpdate memory lockUpdate = LockUpdate(updateBatchIndex, _updateLock(lock, _amount, _lockingDuration));
queuedLockUpdates[_lockId].push(lockUpdate);
queuedRESDLSupplyChange +=
int256(lockUpdate.lock.amount + lockUpdate.lock.boostAmount) -
int256(lock.amount + lock.boostAmount);
if (updateNeeded == 0) updateNeeded = 1;
emit QueueUpdateLock(_owner, _lockId, lockUpdate.lock.amount, lockUpdate.lock.boostAmount, lockUpdate.lock.duration);
}
Once a user has queued too many updates, when the keeper send it to the primary chain and he is allowed to execute the updates, he will run out of gas because the function will loop through the whole array
function _executeQueuedLockUpdates(address _owner, uint256[] memory _lockIds) internal updateRewards(_owner) {
uint256 finalizedBatchIndex = _getFinalizedUpdateBatchIndex();
for (uint256 i = 0; i < _lockIds.length; ++i) {
uint256 lockId = _lockIds[i];
_onlyLockOwner(lockId, _owner);
uint256 numUpdates = queuedLockUpdates[lockId].length;
Lock memory curLockState = locks[lockId];
uint256 j = 0;
while (j < numUpdates) {
...
...
++j;
}
for (uint256 k = 0; k < numUpdates; ++k) {
if (j == numUpdates) {
queuedLockUpdates[lockId].pop();
} else {
queuedLockUpdates[lockId][k] = queuedLockUpdates[lockId][j];
++j;
}
}
}
}
If this scenario is reached, the user will not be able ever again to change any state of his lock. He will receive the rewards that his lock generates but will not be able to withdraw his funds.
Impact
Medium
Tools Used
Manual review
Recommendations
Add a limit for lock updates in the same index batch. Just as queueing new locks works.
+ uint256 public queuedUpdatesLimit;
function initialize(
string memory _name,
string memory _symbol,
address _sdlToken,
address _boostController,
uint256 _queuedNewLockLimit,
+ uint256 _queuedUpdatesLimit
) public initializer {
__SDLPoolBase_init(_name, _symbol, _sdlToken, _boostController);
updateBatchIndex = 1;
currentMintLockIdByBatch.push(0);
queuedNewLocks.push();
queuedNewLocks.push();
queuedNewLockLimit = _queuedNewLockLimit;
+ queuedUpdatesLimit = _queuedUpdatesLimit;
}
function _queueLockUpdate(
address _owner,
uint256 _lockId,
uint256 _amount,
uint64 _lockingDuration
) internal onlyLockOwner(_lockId, _owner) {
+ uint256 updates;
+ for(uint256 i; i < queuedLockUpdates[_lockId].length; ++i){
+ if (queuedLockUpdates[lockId][i].updateBatchIndex > finalizedBatchIndex) break;
+ ++updates;
+ }
+ if (updates >= queuedUpdatesLimit) revert TooManyQueuedUpdates();
Lock memory lock = _getQueuedLockState(_lockId);
LockUpdate memory lockUpdate = LockUpdate(updateBatchIndex, _updateLock(lock, _amount, _lockingDuration)); // returns lock with _amount increased and lockingDuration as new duration
queuedLockUpdates[_lockId].push(lockUpdate);
queuedRESDLSupplyChange +=
int256(lockUpdate.lock.amount + lockUpdate.lock.boostAmount) -
int256(lock.amount + lock.boostAmount);
if (updateNeeded == 0) updateNeeded = 1;
emit QueueUpdateLock(_owner, _lockId, lockUpdate.lock.amount, lockUpdate.lock.boostAmount, lockUpdate.lock.duration);
}
