## Summary The closeCase function in the RecoverableWrapper deletes the records when the rawIndex is less than the head of the queue without checking the frozen amount of the record. ## Finding Description The [`closeCase`](https://cantina.xyz/code/26d5255b-6f68-46cf-be55-81dd565d9d16/tn-contracts/src/recoverable-wrapper/RecoverableWrapper.sol?lines=578,578) function in the `RecoverableWrapper` contract is called by the owner when they want to close a frozen amount for a given account. The function retrieves the head of the `unsettledRecord` queue for the account. If the `Suspension.rawIndex` is less than the head, the function calls the `unfreezeRecord` function, which deletes the user's record at the specified `rawIndex`. ```soliditty function closeCase( bool recover, address victim, Suspension[] memory freezes ) external virtual onlyOwner returns (bool) { for (uint256 i = 0; i < freezes.length; i++) { uint256 rawIndex = freezes[i].rawIndex; address account = freezes[i].account; uint128 amount = freezes[i].amount; Record memory r = _unsettledRecords[account].getAt(rawIndex); if (r.settlementTime == 0) { revert RecordNotFound(account, rawIndex); } uint256 head = _unsettledRecords[account].head; // checks that there is enough frozen to unfreeze. only deletes if past settlement time and is before head. // should also delete if queue is empty @> bool del = rawIndex < head || _unsettledRecords[account].isEmpty(); _unsettledRecords[account].unfreezeRecord(rawIndex, amount, del); frozen[account] -= amount; // remove from cachedUnsettledFrozen if (rawIndex <= _unsettledRecords[account].cacheIndex) { _accountState[account].cachedUnsettledFrozen -= amount; } if (recover) { // spend the now-unfrozen record if (!del) { _unsettledRecords[account].decrementRecordAmount(rawIndex, amount); if (rawIndex <= _unsettledRecords[account].cacheIndex) { _accountState[account].cachedUnsettled -= amount; } } uint256 toRawIndex = _unsettledRecords[victim].enqueue(amount, block.timestamp + recoverableWindow); _accountState[account].balance -= amount; _accountState[victim].balance += amount; _accountState[account].nonce++; _accountState[victim].nonce++; bool pastSettlement = r.settlementTime <= block.timestamp; emit Transfer(account, victim, pastSettlement ? 0 : amount, pastSettlement ? amount : 0, toRawIndex); } } emit CaseClosed(recover, victim, freezes); return true; } ``` The issue arises when the owner attempts to close only part of a record’s amount. In such cases, the entire record may still get deleted due to the `del` flag being `true`, even though a portion of the amount remains. This leads to the account being able to unwrap the frozen amount prematurely, and causes the `frozen` balance of the account to reflect an incorrect (lower) value. ```solidity function unfreezeRecord(RecordsDeque storage rd, uint256 rawIndex, uint128 unfreezeAmount, bool del) public { uint256 frozen = rd.queue[rawIndex].frozen; if (unfreezeAmount > frozen) { revert InvalidUnfreezeAmount(frozen, unfreezeAmount); } if (del) { delete rd.queue[rawIndex]; } else { rd.queue[rawIndex].frozen -= unfreezeAmount; } } ``` ## Impact Explanation Impact is High because a user can withdraw the entire frozen amount even if the owner has not fully closed it. ## Likelihood Explanation Likelihood is High because the owner can easily make the mistake of not closing the entire frozen amount. ## Proof of Concept 1. InitialState: owner freeze's 50e18 at 1st and 2nd index , queue = [{100e18, 1000, 50e18, 0, 2}, {150e18, 1500, 50e18, 1, 3}, {200e18, 2000, 0, 2, 4}, {250e18, 2500, 0, 3, 0}], _unsettledRecords[User1] = {queue, head = 1, tail = 4, cacheIndex = 0}. 2. At the 1501 User1 tries to unwraps 150e18. 3. queue = [{100e18, 1000, 50e18, 0, 2}, {150e18, 1500, 50e18, 0, 3}, {200e18, 2000, 0, 0, 4}, {250e18, 2500, 0, 3, 0}], _unsettledRecords[User1 = {queue, head =3, tail = 4, cacheIndex = 4}. 4. Now, the owner tries to closeCase(Suspension[{user1, 1 ,50e18},{user1, 2, 25e18}]. 5. For the 1st index the rawIndex is less than head (1<4) so the 1st Index record is deleted. But we can see that for 2nd index the amount is 25e18 still the record gets deleted. ## Recommendation Make sure that there is a check that makes sure the amount is equal to the frozen amount. ```diff function closeCase( bool recover, address victim, Suspension[] memory freezes ) external virtual onlyOwner returns (bool) { for (uint256 i = 0; i < freezes.length; i++) { uint256 rawIndex = freezes[i].rawIndex; address account = freezes[i].account; uint128 amount = freezes[i].amount; Record memory r = _unsettledRecords[account].getAt(rawIndex); if (r.settlementTime == 0) { revert RecordNotFound(account, rawIndex); } uint256 head = _unsettledRecords[account].head; // checks that there is enough frozen to unfreeze. only deletes if past settlement time and is before head. // should also delete if queue is empty - bool del = rawIndex < head || _unsettledRecords[account].isEmpty(); + bool del = (( rawIndex < head || _unsettledRecords[account].isEmpty() ) && amount == r.frozen); _unsettledRecords[account].unfreezeRecord(rawIndex, amount, del); frozen[account] -= amount; // remove from cachedUnsettledFrozen if (rawIndex <= _unsettledRecords[account].cacheIndex) { _accountState[account].cachedUnsettledFrozen -= amount; } if (recover) { // spend the now-unfrozen record if (!del) { _unsettledRecords[account].decrementRecordAmount(rawIndex, amount); if (rawIndex <= _unsettledRecords[account].cacheIndex) { _accountState[account].cachedUnsettled -= amount; } } uint256 toRawIndex = _unsettledRecords[victim].enqueue(amount, block.timestamp + recoverableWindow); _accountState[account].balance -= amount; _accountState[victim].balance += amount; _accountState[account].nonce++; _accountState[victim].nonce++; bool pastSettlement = r.settlementTime <= block.timestamp; emit Transfer(account, victim, pastSettlement ? 0 : amount, pastSettlement ? amount : 0, toRawIndex); } } emit CaseClosed(recover, victim, freezes); return true; } ```

Bizarro

Public reviews