Incorrect accounting of _pendingWithdrawal in queueClaiming flow

[c4-bot-4]

Lines of code

https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/pools/Pool.sol#L678

Vulnerability details

Impact

Incorrect accounting of _pendingWithdrawal in queueClaiming flow, funds received from a previous queue index will be lost.

Proof of Concept

In Pool.sol - queueClaimAll(), each queue's received funds getTotalReceived[_idx] (total returned funds from loans for that queue) will be distributed to all newer queues in a for-loop.

There are two for-loops in this flow. First for-loop iterate through each pendingWithdrawal index to get the received funds for that queue index (getTotalReceived[_idx]). Second for-loop iterates through each queue index again to distribute funds from _idx.

The problem is in the second for-loop, _pendingWithdrawal[secondIdx] will not accumulate distributed funds from previous queue indexes, instead it erases the value from previous loops and only records the last queue's received funds.

//src/lib/pools/Pool.sol
    function _updatePendingWithdrawalWithQueue(
        uint256 _idx,
        uint256 _cachedPendingQueueIndex,
        uint256[] memory _pendingWithdrawal
    ) private returns (uint256[] memory) {
        uint256 totalReceived = getTotalReceived[_idx];
        uint256 totalQueues = getMaxTotalWithdrawalQueues + 1;
...
        getTotalReceived[_idx] = 0;
...
        for (uint256 i; i < totalQueues;) {
...
              //@audit this should be _pendingWithdraw[secondIdx] += pendingForQueue; Current implementation directly erases `pendingForQueue` value distributed from other queues. 
|>            _pendingWithdrawal[secondIdx] = pendingForQueue;
...

(https://github.com/code-423n4/2024-04-gondi/blob/b9863d73c08fcdd2337dc80a8b5e0917e18b036c/src/lib/pools/Pool.sol#L678) Note that getTotalReceived[_idx] will be cleared before the for-loop(getTotalReceived[_idx] = 0), meaning that the erased pendingForQueue values from previous loops cannot be recovered. _pendingWithdrawal will be incorrect.

Tools Used

Manual

Recommended Mitigation Steps

Change into _pendingWithdrawal[secondIdx] + = pendingForQueue;

Assessed type

Error

[c4-judge]

0xA5DF marked the issue as primary issue

[c4-judge]

0xA5DF marked the issue as selected for report

[c4-judge]

0xA5DF marked the issue as satisfactory

[0xend]

https://github.com/pixeldaogg/florida-contracts/pull/374