Skip to content
This repository has been archived by the owner on Sep 17, 2023. It is now read-only.

yixxas - Collateral ratio is computed wrongly for collaterals that are not 18 decimals #17

Closed
sherlock-admin opened this issue Mar 13, 2023 · 0 comments
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue

Comments

@sherlock-admin
Copy link
Contributor

sherlock-admin commented Mar 13, 2023

yixxas

high

Collateral ratio is computed wrongly for collaterals that are not 18 decimals

Summary

Collateral ratio is being computed as the number of decimals a collateral has, which is wrongly compared with MIN_COL_RATIO, which is fixed at 18 decimals.

Vulnerability Detail

Collateral ratio is computed in this way.

uint256 newCollRatio = (_coll * _price * Constants.PRECISION) / (_debt * 10 ** priceDecimals)

We know that number of decimals in _price and 10 ** priceDecimals will cancel each other out. Because _debt is in TAU tokens, it is 18 decimals. Constants.PRECISION is also 18 decimals hence they will always cancel out. The number of decimals in the resulting newCollRatio would be dependant on number of decimals in _coll, our collateral.

    function _computeCR(
        uint256 _coll,
        uint256 _debt,
        uint256 _price,
        uint8 priceDecimals
    ) internal pure returns (uint256) {
        if (_debt > 0) {
            uint256 newCollRatio = (_coll * _price * Constants.PRECISION) / (_debt * 10 ** priceDecimals);

            return newCollRatio;
        }
        // Return the maximal value for uint256 if the account has a debt of 0. Represents "infinite" CR.
        else {
            // if (_debt == 0)
            return type(uint256).max;
        }
    }
}

This collateral ratio is used to check if an account is considered healthy in _calcLiquidationDiscount().

    function _calcLiquidationDiscount(uint256 _accountHealth) internal pure returns (uint256 liquidationDiscount) {
        if (_accountHealth >= MIN_COL_RATIO) {
            revert cannotLiquidateHealthyAccount();
        }

        // The liquidator's discount on user funds is based on how far underwater the position is, to simulate a dutch auction.
        // The discount is capped at MAX_LIQ_DISCOUNT.
        uint256 diff = (MIN_COL_RATIO + LIQUIDATION_SURCHARGE) - _accountHealth;
        if (diff > MAX_LIQ_DISCOUNT) {
            diff = MAX_LIQ_DISCOUNT;
        }

        liquidationDiscount = Constants.PRECISION + diff;
    }

We see that _accountHealth which is our collateral ratio is being compared with the constant MIN_COL_RATIO, which is fixed at 18 decimals. However, our computed collateral ratio is based on the number of decimals in the used collateral.

Impact

This is dangerous as, if collateral has < 18 decimals, than it means that the check if an account is healthy will be passed far more frequently that not; that is an account will be constantly being wrongly considered healthy, hence allowing users to take on more debt than they should be allowed to.

On the other hand, a if collateral > 18 decimals, account will be far more frequently considered unhealthy, preventing any meaningful debt to be taken for the deposited collateral.

Code Snippet

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Libs/TauMath.sol#L18
https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L432-L445

Tool used

Manual Review

Recommendation

Consider scaling the output of _computeCR to 18 decimals as it is being compared with a constant MIN_COL_RATIO which is in 18 decimals.

Duplicate of #35

@github-actions github-actions bot added High A valid High severity issue Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label labels Mar 21, 2023
@sherlock-admin sherlock-admin added the Reward A payout will be made for this issue label Apr 1, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate A valid issue that is a duplicate of an issue with `Has Duplicates` label High A valid High severity issue Reward A payout will be made for this issue
Projects
None yet
Development

No branches or pull requests

1 participant