sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

dipp - ```_getTotalReservesAndCollaterals``` may not account for all tokens deposited as collateral in the ```InsurancePool``` contract #142

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

dipp

medium

_getTotalReservesAndCollaterals may not account for all tokens deposited as collateral in the InsurancePool contract

Summary

Guardians are allowed to deposit collateral into the InsurancePool without the token being listed in the TokenManager leading to the _getTotalReservesAndCollaterals in the Unitas.sol contract returning a lower amount for collateral than it should.

Vulnerability Detail

In the _getTotalReservesAndCollaterals function in Unitas.sol only tokens listed by TokenManager are accounted for. When the getCollateral function of the InsurancePool is called, only tokens added in TokenManager will be considered.

Unitas.sol#L500-L535:

    function _getTotalReservesAndCollaterals() internal view returns (uint256 reserves, uint256 collaterals) {
        address baseToken = address(tokenManager.usd1());
        uint8 tokenTypeValue = uint8(ITokenManager.TokenType.Asset);
        uint256 tokenCount = tokenManager.tokenLength(tokenTypeValue);
        uint256 priceBase = 10 ** oracle.decimals();

        for (uint256 i; i < tokenCount; i++) {
            address token = tokenManager.tokenByIndex(tokenTypeValue, i);
            uint256 tokenReserve = _getBalance(token);
            uint256 tokenCollateral = IInsurancePool(insurancePool).getCollateral(token);

            if (tokenReserve > 0 || tokenCollateral > 0) {
                uint256 price = oracle.getLatestPrice(token);

                reserves += _convert(
                    token,
                    baseToken,
                    tokenReserve,
                    MathUpgradeable.Rounding.Down,
                    price,
                    priceBase,
                    token
                );

                collaterals += _convert(
                    token,
                    baseToken,
                    tokenCollateral,
                    MathUpgradeable.Rounding.Down,
                    price,
                    priceBase,
                    token
                );
            }
        }
    }

However, the depositCollateral function in InsurancePool.sol allows the Guardian to deposit any token without requiring it to have been added to the TokenManager contract.

InsurancePool.sol#L83-L91:

    function depositCollateral(address token, uint256 amount) external onlyGuardian nonReentrant {
        _checkAmountPositive(amount);

        _setBalance(token, _getBalance(token) + amount);

        IERC20(token).safeTransferFrom(msg.sender, address(this), amount);

        emit CollateralDeposited(token, msg.sender, amount);
    }

The inconsistency between the tokens deposited into the InsurancePool and the added tokens in the TokenManager may lead to the _getTotalReservesAndCollaterals function not counting all collateral held in the InsurancePool.

Impact

_getTotalReservesAndCollaterals may return a lower value than it should which may cause _checkReserveRatio to revert if the reserveRatio returned by _getTotalReservesAndCollaterals is less than the reserveRatioThreshold.

This could lead to some swaps reverting unfairly.

Additionally, external protocols integrating with Unitas may suffer due to the incorrect getReserveStatus return value.

Code Snippet

InsurancePool:depositCollateral#L83-L91

Unitas.sol:_getTotalReservesAndCollaterals#L500-L535

Tool used

Manual Review

Recommendation

In the depositCollateral function in InsurancePool.sol, consider adding a check to make sure the token has been added in the TokenManager.