sashik_eth - DOS of withdrawing assets if IP does not have enough reserves

[sherlock-admin]

sashik_eth

medium

DOS of withdrawing assets if IP does not have enough reserves

Summary

DOS of withdrawing assets if IP does not have enough reserves.

Vulnerability Detail

Protocol documentation states that users should be able to withdraw assets even if the reserve ratio falls to less than 100%. Reserve ratio accounting includes both - reserve on Unitas.sol itself and in Insurance Pool:

File: Unitas.sol
500:     function _getTotalReservesAndCollaterals() internal view returns (uint256 reserves, uint256 collaterals) {
501:         address baseToken = address(tokenManager.usd1());
502:         uint8 tokenTypeValue = uint8(ITokenManager.TokenType.Asset);
503:         uint256 tokenCount = tokenManager.tokenLength(tokenTypeValue);
504:         uint256 priceBase = 10 ** oracle.decimals(); // 18
505: 
506:         for (uint256 i; i < tokenCount; i++) {
507:             address token = tokenManager.tokenByIndex(tokenTypeValue, i);
508:             uint256 tokenReserve = _getBalance(token);
509:             uint256 tokenCollateral = IInsurancePool(insurancePool).getCollateral(token);
510: 
511:             if (tokenReserve > 0 || tokenCollateral > 0) {
512:                 uint256 price = oracle.getLatestPrice(token);
513: 
514:                 reserves += _convert(
515:                     token,
516:                     baseToken,
517:                     tokenReserve,
518:                     MathUpgradeable.Rounding.Down,
519:                     price,
520:                     priceBase,
521:                     token
522:                 );
523: 
524:                 collaterals += _convert(
525:                     token,
526:                     baseToken,
527:                     tokenCollateral,
528:                     MathUpgradeable.Rounding.Down,
529:                     price,
530:                     priceBase,
531:                     token
532:                 );
533:             }
534:         }
535:     }

This means that in case the reserve ratio is < 100% - the sum of all collateral in both contracts would be less than the minted USDx tokens value. This would lead to DOS withdrawing user assets here, since _swapOut would revert to trying to withdraw from IP more collateral than it has:

File: Unitas.sol
379:     function _swapOut(address token, address receiver, uint256 amount) internal {
380:         ITokenManager.TokenType tokenType = tokenManager.getTokenType(token);
381: 
382:         require(tokenType != ITokenManager.TokenType.Undefined);
383: 
384:         if (tokenType == ITokenManager.TokenType.Asset) {
385:             uint256 tokenReserve = _getBalance(token);
386:             uint256 reserveAmount = amount.min(tokenReserve - _getPortfolio(token));
387: 
388:             if (amount > reserveAmount) {
389:                 uint256 collateralAmount = amount - reserveAmount;
390: 
391:                 // Pull the collateral from insurance pool
392:                 IInsurancePool(insurancePool).withdrawCollateral(token, collateralAmount); 
393:             }
394: 
395:             _setBalance(token, tokenReserve - reserveAmount);
396:             IERC20(token).safeTransfer(receiver, amount);
397:         } else {
398:             IERC20Token(token).mint(receiver, amount);
399:         }
400:     }
401: 

Impact

Users would not be able to withdraw collaterals if the reserve ratio is less than 100%.

Code Snippet

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

Tool used

Manual Review

Recommendation

Consider allowing the withdrawal of all left funds in reserves even if reserve ratio is less 100%. This would guarantee that users would be able to withdraw at least some part of the collateral and these funds would not be locked on a Unitas.sol contract until the IP balance would be updated.

Duplicate of #95

[sashik-eth]

Escalate for 10 USDC Protocol documentation clearly states that users should be able to withdraw tokens in case the reserve ratio is < 100%. Consider a scenario when the protocol holds 800 USDT in Unitas contract and 100 USDT in InsurancePool, at the same time user holds USD1 tokens for the equivalent value of 1000 USDT. This means that the reserve ratio is 90%. When user would try to withdraw all USDT - tx would revert since the _swapOut function would call IInsurancePool#withdrawCollateral(Line 392) with an amount of 200 USDT while InsurancePool holds only 100 USDT.

[sherlock-admin]

Escalate for 10 USDC Protocol documentation clearly states that users should be able to withdraw tokens in case the reserve ratio is < 100%. Consider a scenario when the protocol holds 800 USDT in Unitas contract and 100 USDT in InsurancePool, at the same time user holds USD1 tokens for the equivalent value of 1000 USDT. This means that the reserve ratio is 90%. When user would try to withdraw all USDT - tx would revert since the _swapOut function would call IInsurancePool#withdrawCollateral(Line 392) with an amount of 200 USDT while InsurancePool holds only 100 USDT.

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.

[ctf-sec]

The report describes a intended behavior, not an attack

[jacksanford1]

Seems like a known risk that the protocol could go undercollateralized (<100% reserve ratio), so that is expected. And in that scenario, it's true that a user would need to input a value that is less than or equal to the amount of reserves left in the protocol in order for a withdrawal to succeed. Inputting an amount greater may lead to a revert (haven't verified this fully) and agree that it would be better if instead it simply withdrew whatever was left in the reserves. This could not constitute a Medium issue because the revert can simply be fixed by decreasing the amount to withdraw.

@sashik-eth Let me know if you have any additional evidence to support a Medium here.

[jacksanford1]

Based on this line in the whitepaper:

The Unitas protocol guarantees unrestricted and unconditional conversion of its unitized stablecoins “back” to USD-pegged stablecoins.

I think this issue needs to be duped into the larger category of "breaks unconditional exit" issues.

[jacksanford1]

Escalation accepted

Duplicate of #95

[hrishibhat]

Result: Medium Duplicate of #95

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• sashik-eth: accepted