Last withdrawal would be blocked

Summary

Last withdrawal would be blocked.

Vulnerability Detail

At the end of the swap() execution Unitas.sol checks the current reserve ratio and reserve status and reverts swaps in case these values are wrong:

File: Unitas.sol
605:     function _checkReserveRatio(uint232 reserveRatioThreshold) internal view {
606:         if (reserveRatioThreshold == 0) {
607:             return;
608:         } else {
609:             (uint256 reserves, uint256 collaterals) = _getTotalReservesAndCollaterals();
610:             uint256 allReserves = reserves + collaterals;
611:             uint256 liabilities = _getTotalLiabilities();
612: 
613:             (ReserveStatus reserveStatus, uint256 reserveRatio) = _getReserveStatus(allReserves, liabilities);
614: 
615:             if (reserveStatus != ReserveStatus.Infinite) { 
616:                 _require(reserveRatio > reserveRatioThreshold, Errors.RESERVE_RATIO_NOT_GREATER_THAN_THRESHOLD);
617:             }
618:         }
619:     }

reserveStatus could have 3 values:

Undefined when allReserves and liabilities are zero
Infinite when liabilities is zero
Otherwise Finite

So the _checkReserveRatio() function would check the reserveRatio value in 2 cases - when allReserves and liabilities are both zero or non-zero.

The second case would take place most time of protocol existence, while the first case could take place when the last token holder would be withdrawing collateral and IP reserves would be equal to 0 (because of protocol become abandoned for example). In such a scenario at the end of the swap execution allReserves and liabilities would be equal to 0 and the transaction would be reverted due to not enough reserveRatio in line 616.

Basically it would create a situation when the contract has enough collateral (1 to 1 to user's tokens) but withdrawing would be impossible.

Impact

Last withdrawer would not be able to withdraw all left collateral from the protocol.

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/Unitas.sol#L615-L617

Tool used

Manual Review

Recommendation

Consider updating the reserve ratio check to the next:

-           if (reserveStatus != ReserveStatus.Infinite) { 
+           if (reserveStatus == ReserveStatus.Finite) { 
                _require(reserveRatio > reserveRatioThreshold, Errors.RESERVE_RATIO_NOT_GREATER_THAN_THRESHOLD);
            }

sashik-eth commented 1 year ago

Escalate for 10 USDC This is a valid medium since the last holder of the usd1 tokens would be unable to withdraw collateral while protocol holds enough (100%) collateral for this.

sherlock-admin commented 1 year ago

Escalate for 10 USDC This is a valid medium since the last holder of the usd1 tokens would be unable to withdraw collateral while protocol holds enough (100%) collateral for this.

You've created a valid escalation for 10 USDC!

To remove the escalation from consideration: Delete your comment.

You may delete or edit your escalation comment anytime before the 48-hour escalation window closes. After that, the escalation becomes final.

0xruhum commented 1 year ago

Generally, the issue has to result in a "material amount of funds being lost" to be a valid med/high. https://docs.sherlock.xyz/audits/judging/judging

In this case, the user could just leave 1 wei inside the reserves so that the reserve status is "Infinite". That way they can withdraw 99.9% of their funds.