Category: Coding Mistakes
Incorrect loop implementation in the function clearQueuedDeposits
Low Severity
Low Impact
Medium Likelihood
Description
The function clearQueuedDeposits is used to clear a fixed amount of deposits in the queue. This function loops through the queueDeposits mapping and pops the last element of the array while minting shares to the expected receivers in that mapping.
The issue is that the array indexing used to access queueDeposits is incorrect because the array index will be out of bound in many cases. Shown below is the relevant part of the code:
function clearQueuedDeposits(
uint256 queueSize
) external onlyOwner returns (uint256 pulledAmount) {
//...
for (uint256 i = depositLength - queueSize; i < queueSize; ) {
QueueDeposit memory qDeposit = queueDeposits[queueSize - i - 1];
uint256 shares = qDeposit.assets.mulDivDown(
cachedSupply,
cachedAssets
);Impact
In many cases the function might revert.
Recommendations
Consider changing the code to the following:
function clearQueuedDeposits(
uint256 queueSize
) external onlyOwner returns (uint256 pulledAmount) {
//...
+ for (uint256 i = depositLength; i > depositLength - queueSize; ) {
+ QueueDeposit memory qDeposit = queueDeposits[i - 1];
//...
+ unchecked {
+ i--;
+ }Remediation
The issue was fixed in commit cf415dd↗.