The protocol has the functionallity to allow users to automatically roll their positions over to the next available epoch. This functionallity is implemented by a relayer calling the mintRollovers function. For a given epoch, the starting index of where to begin iterating through the rollOverQueue is defined here. However, if a user that has already had their position rolled over to the next epoch calls the delistInRollover function, and the rollOverQueue has not been iterated through completely, the user who owns the position last in the rollOverQueue will have their position swapped to the index of the user's position that is delisting and, then, will not have their position rolled over to the next epoch.
Vulnerability Detail
If the rollOverQueue has not been completely iterated through and a user that has had their position rolled over to the next epoch invokes the delistInRollover function on their position in the queue, the user that has their position swapped with the user's position that is delisting will not have their position rolled over to the next epoch. This is due to the fact that the mintRollOvers function iterates through the rollOverQueue beginning at the index defined here:
372 uint256 index = rolloverAccounting[_epochId];
Once the mintRollOvers function has iterated through the amount of positions defined by the 'operations' argument, the index is updated here:
453 if (executions > 0) rolloverAccounting[_epochId] = index;
Given the queue has not been iterated through completely for the respective epoch, if a user with a position in the rollOverQueue that is less than the epoch's respective index (meaning their position has been rolled over to the next epoch or iterated passed) delists their position by calling the delistInRollover function, the user that currently has their position last in the queue will not have their position rolled over to the next epoch because their position in the queue will be updated to the user's positon in the queue that invoked the delistInRollover function. This functionallity is shown here:
294 rolloverQueue[index] = rolloverQueue[length - 1];
295 // remove the last item in the queue
296 rolloverQueue.pop();
297 // update the index of prev last user ( mapping index is allways array index + 1)
298 ownerToRollOverQueueIndex[rolloverQueue[index].receiver] =
299 index +
300 1;
301 // remove receiver from index mapping
302 delete ownerToRollOverQueueIndex[_owner];
Because the respective rolloverQueue index of the user who had their position swapped to the new index in the queue is, now, less than the index defined by 'rolloverAccounting[_epochId]' for the next epoch, this user's position will not be rolled over to the respective epoch because this index in the queue has already been iterated passed.
Impact
Users will experience a DOS for the automatic roll-over functionality and their funds will be idle.
Code Snippet
Please see the links and mentioned blocks of code above for the affected code.
Tool used
Manual Review
Recommendation
As a mitigation for this, it is recommended to not allow a user to call the delistInRollover function if their position has already been rolled over to the next epoch. This can be done by implementing the following check with a respective custom error in the delistInRollover function on line 287 between the definitions of the 'index' and 'length' variables:
BPZ
medium
The mintRollovers function will skip users
BPZ
medium
Summary
The protocol has the functionallity to allow users to automatically roll their positions over to the next available epoch. This functionallity is implemented by a relayer calling the mintRollovers function. For a given epoch, the starting index of where to begin iterating through the rollOverQueue is defined here. However, if a user that has already had their position rolled over to the next epoch calls the delistInRollover function, and the rollOverQueue has not been iterated through completely, the user who owns the position last in the rollOverQueue will have their position swapped to the index of the user's position that is delisting and, then, will not have their position rolled over to the next epoch.
Vulnerability Detail
If the rollOverQueue has not been completely iterated through and a user that has had their position rolled over to the next epoch invokes the delistInRollover function on their position in the queue, the user that has their position swapped with the user's position that is delisting will not have their position rolled over to the next epoch. This is due to the fact that the mintRollOvers function iterates through the rollOverQueue beginning at the index defined here:
Once the mintRollOvers function has iterated through the amount of positions defined by the 'operations' argument, the index is updated here:
Given the queue has not been iterated through completely for the respective epoch, if a user with a position in the rollOverQueue that is less than the epoch's respective index (meaning their position has been rolled over to the next epoch or iterated passed) delists their position by calling the delistInRollover function, the user that currently has their position last in the queue will not have their position rolled over to the next epoch because their position in the queue will be updated to the user's positon in the queue that invoked the delistInRollover function. This functionallity is shown here:
Because the respective rolloverQueue index of the user who had their position swapped to the new index in the queue is, now, less than the index defined by 'rolloverAccounting[_epochId]' for the next epoch, this user's position will not be rolled over to the respective epoch because this index in the queue has already been iterated passed.
Impact
Users will experience a DOS for the automatic roll-over functionality and their funds will be idle.
Code Snippet
Please see the links and mentioned blocks of code above for the affected code.
Tool used
Manual Review
Recommendation
As a mitigation for this, it is recommended to not allow a user to call the delistInRollover function if their position has already been rolled over to the next epoch. This can be done by implementing the following check with a respective custom error in the delistInRollover function on line 287 between the definitions of the 'index' and 'length' variables:
Duplicate of #72