sherlock-audit / 2023-04-unitasprotocol-judging

4 stars 3 forks source link

mrpathfindr - _getTotalLiabilities does not check if baseToken has been set yet leading to _checkReserveRatio failing. #139

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

mrpathfindr

medium

_getTotalLiabilities does not check if baseToken has been set yet leading to _checkReserveRatio failing.

Summary

The function _getTotalLiabilities is used to check the reserve ratio of the protocol through _getReserveStatus. The reserve status can be set to infinite if liabilities are 0. (Which is possible if baseToken is not set). This means the function _checkReserveRatio will produce incorrect outcomes leading to users being able to swap tokens at unwanted periods.

Vulnerability Detail

Let's examine the function _getTotalLiabilities


 function _getTotalLiabilities() internal view returns (uint256 liabilities) {
        address baseToken = address(tokenManager.usd1());
        uint8 tokenTypeValue = uint8(ITokenManager.TokenType.Stable);
        uint256 tokenCount = tokenManager.tokenLength(tokenTypeValue);
        uint256 priceBase = 10 ** oracle.decimals();

        for (uint256 i; i < tokenCount; i++) {
            address token = tokenManager.tokenByIndex(tokenTypeValue, i);
            uint256 tokenSupply = IERC20Token(token).totalSupply();

            if (token == baseToken) {
                // Adds up directly when the token is USD1
                liabilities += tokenSupply;
            } else if (tokenSupply > 0) {
                uint256 price = oracle.getLatestPrice(token);

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

Similar to _getPriceQuoteToken there should be a require clause, ensuring the base token has been set (i.e baseToken != address(0)`

Impact

If the base token (usd1) has not been set (i.e the address of base == address(0) _getTotalLiabilities will return 0, therefore, when its value is being used to obtain the reserve status, this code block will be executed from _getReserveStatus.


    {
        if (liabilities == 0) {
            reserveStatus = allReserves == 0 ? ReserveStatus.Undefined : ReserveStatus.Infinite;

When this code block is executed and allReserves != 0. ReserveStatus.Infinite is true, and users will be able to swap any token pair at this point due to _checkReserveRatio failing to catch the correct reserve ratio.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

I recommend including this requirement to _getTotalLiabilities to mitigate the possibility of false positive reserve ratio checks.


 function _getTotalLiabilities() internal view returns (uint256 liabilities) {
        _require(baseToken != address(0), Errors.USD1_NOT_SET);
        address baseToken = address(tokenManager.usd1());
        uint8 tokenTypeValue = uint8(ITokenManager.TokenType.Stable);
        uint256 tokenCount = tokenManager.tokenLength(tokenTypeValue);
        uint256 priceBase = 10 ** oracle.decimals();

        for (uint256 i; i < tokenCount; i++) {
            address token = tokenManager.tokenByIndex(tokenTypeValue, i);
            uint256 tokenSupply = IERC20Token(token).totalSupply();

            if (token == baseToken) {
                // Adds up directly when the token is USD1
                liabilities += tokenSupply;
            } else if (tokenSupply > 0) {
                uint256 price = oracle.getLatestPrice(token);

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