ast3ros - In case the portfolio makes a loss, the total reserves and reserve ratio will be inflated.

[sherlock-admin]

ast3ros

medium

In case the portfolio makes a loss, the total reserves and reserve ratio will be inflated.

Summary

The pool balance is transferred to the portfolio for investment, for example sending USDT to Curve/Aave/Balancer etc. to generate yield. However, there are risks associated with those protocols such as smart contract risks. In case a loss happens, it will not be reflected in the pool balance and the total reserve and reserve ratio will be inflated.

Vulnerability Detail

The assets in the pool can be sent to the portfolio account to invest and earn yield. The amount of assets in the insurance pool and Unitas pool is tracked by the _balance variable. This amount is used to calculate the total reserve and total collateral, which then are used to calculate the reserve ratio.

        uint256 tokenReserve = _getBalance(token);
        uint256 tokenCollateral = IInsurancePool(insurancePool).getCollateral(token);

When there is a loss to the portfolio, there is no way to write down the _balance variable. This leads to an overstatement of the total reserve and reserve ratio.

Impact

Overstatement of the total reserve and reserve ratio can increase the risk for the protocol because of undercollateralization of assets.

Code Snippet

https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/Unitas.sol#L508-L509 https://github.com/sherlock-audit/2023-04-unitasprotocol/blob/main/Unitas-Protocol/src/PoolBalances.sol#L111-L113

Tool used

Manual Review

Recommendation

Add function to allow admin to write off the _balance in case of investment lost. Example:

function writeOff(address token, uint256 amount) external onlyGuardian {

    uint256 currentBalance = IERC20(token).balanceOf(address(this));

    // Require that the amount to write off is less than or equal to the current balance
    require(amount <= currentBalance, "Amount exceeds balance");
    _balance[token] -= amount;

    emit WriteOff(token, amount);
}

[Adityaxrex]

Aditya: This will need to be manually updated. The likelyhood is small but can be contained by diversification and risk management.

[SunXrex]

in phase 1, reserve ratio will not impact by portfolio impact

[ctf-sec]

This will need to be manually updated. The likelyhood is small but can be contained by diversification and risk management.

Intend to maintain the medium severity

[0xffff11]

I agree with a medium. Despite being very unlikely, the risk exists.

[Shogoki]

Escalate for 10 USDC I think this should be rated as low, as per the sponsors comments. I think it is similiar to #95 the design decision to use the portfolio together with the collateral & accepted risk of the protocol.

[sherlock-admin]

Escalate for 10 USDC I think this should be rated as low, as per the sponsors comments. I think it is similiar to #95 the design decision to use the portfolio together with the collateral & accepted risk of the protocol.

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.

[thangtranth]

Escalate for 10 USDC

@Shogoki thank you for your idea. However those are two different issues:

In #95, the sponsors explained their design choice of “taking a small portion to invest in delta neutral defi pools.” This implies that there are enough funds for users to withdraw at any time.

In this issue, the sponsors mention above how to “contain” the risks, meaning how the protocol will try to reduce the financial loss in case of a negative event. However, the loss still happens and the reserve accounting of the protocol contract is incorrect. Therefore this is a valid medium issue.

[sherlock-admin]

Escalate for 10 USDC

@Shogoki thank you for your idea. However those are two different issues:

In #95, the sponsors explained their design choice of “taking a small portion to invest in delta neutral defi pools.” This implies that there are enough funds for users to withdraw at any time.

In this issue, the sponsors mention above how to “contain” the risks, meaning how the protocol will try to reduce the financial loss in case of a negative event. However, the loss still happens and the reserve accounting of the protocol contract is incorrect. Therefore this is a valid medium issue.

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.

[Shogoki]

I think in both cases the sent out fund are not directly available for users to withdraw.

[thangtranth]

No, it is not. Just one simple example that shows the difference:

As design, the protocol is at 130% reserve ratio (110% stays in pool for redemption, 20% for yield generation in portfolio). The portfolio makes a loss, which drive the reserve ratio down to 115% (110% still stays in the pool for redemption, 5% left in portfolio).

In this case, all the funds are still available for users. But the reserve ratio is hugely overstated (130% instead of 115%).

[ctf-sec]

Can keep the medium severity, inflated total reserves can be considered as accounting issue if the invested portfolio make a loss (countless example of rug and hack, certainly we do not hope the protocol, our sponsor deal with such situation)

[jacksanford1]

Agree with Medium. The case for Low is:

I think this should be rated as low, as per the sponsors comments.

Sponsor is not saying it should be low imo. Sponsor is saying it's unlikely, but it can still be a Medium even if that's true.

I think it is similiar to https://github.com/sherlock-audit/2023-04-unitasprotocol-judging/issues/95 the design decision to use the portfolio together with the collateral & accepted risk of the protocol.

It seems like the problem is that a loss of assets would not be properly reflected in the pool balance, resulting in bad outcomes. If the pool balance is not functioning properly, I don't think that's an intentional design decision.

[0xffff11]

It is a complicated issue so I see the case for both. Though I tend more to medium. Imo a medium makes sense on this one.

[jacksanford1]

This falls into the "breaks unconditional exit" category but also points out something novel which is that the _balance variable doesn't reflect a loss in the portfolio.

Valid Medium.

[Adityaxrex]

@jacksanford1 Thank you

[hrishibhat]

Result: Medium Unique

[sherlock-admin]

Escalations have been resolved successfully!

Escalation status:

• thangtranth: accepted
• Shogoki: rejected

[jacksanford1]

Acknowledged by protocol team (won't fix).