bytes032 - Vault will be practically unuseable if collateral token decimals != 18

[sherlock-admin]

bytes032

medium

Vault will be practically unuseable if collateral token decimals != 18

Summary

The smart contract BaseVault is designed to work with any ERC20 non-rebasing tokens as collateral. However, a token whose decimals are not 18 makes it unusable. The issue arises when _getCollRatio calculates the collateral ratio using the formula: ratio = (collateralAmount * collateralPrice * (1e18) * Constants.PRECISION) / (debt * collateralDecimals).

This formula works well when both the collateral and debt tokens are using 18 decimals. However, if the collateral token uses fewer decimals, the ratio calculation will be incorrect, leading to borrowing errors and broken liquidation logic.

Vulnerability Detail

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L124-L128

    function getAccountHealth(address _account) public view returns (bool) {
        uint256 ratio = _getCollRatio(_account);

        return (ratio > MIN_COL_RATIO);
    }

_getCollRatio is used to calculate the ratio of collateral<>debt for the user. For example if you deposit 1 token of collateral and take 1 token of debt, your ratio will be 1:1. Currently, the protocol is using the MIN_COL_RATIO variable to denote if an account is unhealthy using the getAccountHealth function. In practice, this means the protocol is saying something like, if you want to borrow 1 token, you need to have at least 1.2 tokens as collateral.

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L185-L189

    function _checkAccountHealth(address _account) internal view {
        if (!getAccountHealth(_account)) {
            revert insufficientCollateral();
        }
    }

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L280-L286

    function _modifyPosition(
        address _account,
        uint256 _collateralDelta,
        uint256 _debtDelta,
        bool _increaseCollateral,
        bool _increaseDebt
    ) internal virtual

_checkAccountHealth is used by modifyPosition, which is used to modify user collateral and debt in any way. If debt is increased or collateral reduced, the functionis used to ensure that the account is healthy at the end of the tx.

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L192-L199

function _getCollRatio(address _account) internal view returns (uint256 ratio) {
        // Fetch the price from oracle manager
        (uint256 price, uint8 decimals) = _getCollPrice();
        // Check that user's collateral ratio is above minimum healthy ratio
        ratio = TauMath._computeCR(userDetails[_account].collateral, userDetails[_account].debt, price, decimals);
    }

Back to getCollRatio, we see that it fetches the price and the decimals for the collateral token. Then, it computes the ratio using TauMath._computeCR. And that's where the issue arises.

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Libs/TauMath.sol#L11-L27

    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;
        }
    }

The function is calculating the collateralRatio by using the formula below.

$$ratio = \dfrac{collateralAmount * collateralPrice * (1e18)Constants.PRECISION}{debt * collateralDecimals}$$

The function works as it should, if both tokens are using 18 decimals. However, if the collateral token is using 6 decimals, the formula is broken. Making a real world analogy, Essentially, this means that if the collateral token is USDC, the user needs to deposit 1 200 000 000 000 000 000 USDC to be able to borrow 1 TAU.

Additionally, _getCollRatio also used in
https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L402

function _calcLiquidation

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L135

 function getMaxLiquidation(address _account)

Which will practically break all the liquidation logic too, but I'm not covering that, because we won't get to liquidation scenarios at all given that the borrowing logic is completely shattered.

Here's a PoC in Foundry that demonstrates the issue:

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.13;

import "forge-std/Test.sol";
import "forge-std/console.sol";
import { Ownable } from "@openzeppelin/contracts/access/Ownable.sol";


interface IPriceOracle {
    error NotTrustedAddress();
    error ZeroAddress();
    error InvalidDecimals();
    error notContract();

    event PriceUpdated(address indexed _oracleAddress, uint256 _newPrice, uint256 _timestamp);
    event NodeRegistered(address indexed _oracleAddress, address indexed _nodeAddress);
    event NodeUnRegistered(address indexed _oracleAddress, address indexed _nodeAddress);

    function getLatestPrice(
        bytes calldata _flag
    ) external view returns (uint256 _currentPrice, uint256 _lastPrice, uint256 _lastUpdateTimestamp, uint8 _decimals);
}

contract CustomPriceOracle is IPriceOracle, Ownable {
    string public description;

    uint256 public currentPrice;
    uint256 public lastPrice;
    uint256 public lastUpdateTimestamp;
    uint8 public decimals;

    address public asset;

    uint8 public constant MAX_DECIMALS = 18;

    mapping(address => bool) private trustedNodes;

    modifier isTrusted() {
        if (!trustedNodes[msg.sender]) revert NotTrustedAddress();
        _;
    }

    modifier checkNonZeroAddress(address _addr) {
        if (_addr == address(0)) revert ZeroAddress();
        _;
    }

    constructor(string memory _description, address _underlying, uint8 _decimals) {
        if (_decimals > MAX_DECIMALS) revert InvalidDecimals();

        description = _description;
        decimals = _decimals;
        asset = _underlying;
    }

    function registerTrustedNode(address _node) external checkNonZeroAddress(_node) onlyOwner {
        trustedNodes[_node] = true;
        emit NodeRegistered(address(this), _node);
    }

    function unregisterTrustedNode(address _node) external checkNonZeroAddress(_node) onlyOwner {
        trustedNodes[_node] = false;
        emit NodeUnRegistered(address(this), _node);
    }

    function isTrustedNode(address _node) external view returns (bool) {
        return trustedNodes[_node];
    }

    function updatePrice(uint256 _newPrice) external isTrusted {
        lastPrice = currentPrice;
        currentPrice = _newPrice;


        lastUpdateTimestamp = block.timestamp;

        emit PriceUpdated(address(this), currentPrice, lastUpdateTimestamp);
    }

    function getLatestPrice(
        bytes calldata
    )
        external
        view
        override
        returns (uint256 _currentPrice, uint256 _lastPrice, uint256 _lastUpdateTimestamp, uint8 _decimals)
    {
        return (currentPrice, lastPrice, lastUpdateTimestamp, decimals);
    }
}




contract DecimalsPOC is Test {
    // Taken from BaseVault.sol
    uint256 public constant MIN_COL_RATIO = 1.2e18; // 120 %

    // Taken from Constants,sol
    uint256 internal constant PRECISION = 1e18;

    CustomPriceOracle public oracle;
    address usdcAddress = 0xFF970A61A04b1cA14834A43f5dE4533eBDDB5CC8;
    address daiAddress = 0xDA10009cBd5D07dd0CeCc66161FC93D7c9000da1;
    


    function test18Decimals() public {
        oracle = new CustomPriceOracle("description", daiAddress, 18);
        oracle.registerTrustedNode(address(this));
        oracle.updatePrice(1e18);
        
        uint256 collateral = 1.2e18;
        uint256 debt = 1e18;



        (uint256 _currentPrice, , , uint8 _decimals) = IPriceOracle(
            oracle
        ).getLatestPrice("");
        
        // Deposit 1.2 DAI to get 1 TAU
        assertEq(_computeCR(collateral, debt, _currentPrice, _decimals) == 1.2e18, true);
    }

     function test6Decimals() public {
        oracle = new CustomPriceOracle("description", usdcAddress, 6);
        oracle.registerTrustedNode(address(this));
        oracle.updatePrice(1e6);
        
        uint256 collateral = 1.2e6;
        uint256 debt = 1e18;



        (uint256 _currentPrice, , , uint8 _decimals) = IPriceOracle(
            oracle
        ).getLatestPrice("");
        
        // Deposit 1.2 DAI to get 1 TAU
        assertEq(_computeCR(collateral, debt, _currentPrice, _decimals) == 1.2e18, false);
    }

     function test18DecimalsNew() public {
        oracle = new CustomPriceOracle("description", daiAddress, 18);
        oracle.registerTrustedNode(address(this));
        oracle.updatePrice(1e18);
        
        uint256 collateral = 1.2e18;
        uint256 debt = 1e18;



        (uint256 _currentPrice, , , uint8 _decimals) = IPriceOracle(
            oracle
        ).getLatestPrice("");
        
        // Deposit 1.2 DAI to get 1 TAU
        assertEq(_computeCRNew(collateral, debt, _currentPrice, _decimals) == MIN_COL_RATIO, true);
    }

     function test6DecimalsNew() public {
        oracle = new CustomPriceOracle("description", usdcAddress, 6);
        oracle.registerTrustedNode(address(this));
        oracle.updatePrice(1e6);
        
        uint256 collateral = 1.2e6;
        uint256 debt = 1e18;



        (uint256 _currentPrice, , , uint8 _decimals) = IPriceOracle(
            oracle
        ).getLatestPrice("");
        
        // Deposit 1.2 DAI to get 1 TAU
        assertEq(_computeCRNew(collateral, debt, _currentPrice, _decimals) == MIN_COL_RATIO, true);
    }


    function _computeCR(
        uint256 _coll,
        uint256 _debt,
        uint256 _price,
        uint8 priceDecimals
    ) internal pure returns (uint256) {
        if (_debt > 0) {
            uint256 newCollRatio = (_coll * _price * 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;
        }
    }

    function _computeCRNew(
        uint256 _coll,
        uint256 _debt,
        uint256 _price,
        uint8 priceDecimals
    ) internal pure returns (uint256) {
        if (_debt > 0) {
            if (priceDecimals < 18) {
                uint8 decimalsDifference = 18 - priceDecimals;
                _coll = _coll * (10 ** decimalsDifference);
            }
            uint256 newCollRatio = (_coll * _price * 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;
        }
    }
}

Impact

This vulnerability makes it impossible to use the borrow functionality with tokens whose decimals are not 18. The issue also affects liquidation logic, leading to broken liquidation scenarios.

Code Snippet

https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Libs/TauMath.sol#L11-L27
https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L135
https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L402
https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L192-L199
https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L124-L128
https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L185-L189
https://github.com/sherlock-audit/2023-03-taurus/blob/main/taurus-contracts/contracts/Vault/BaseVault.sol#L280-L286

Tool used

Manual review

Recommendation

To fix the issue, the _computeCR function in TauMath.sol should be updated to handle collateral tokens with decimals that are < 18. One possible fix is to multiply the collateral amount by 10^(18 - collateralDecimals) before computing the collateral ratio. This would adjust the collateral amount to match the expected 18 decimals before computing the ratio.

If the protocol wants to handle tokens with decimals > 18, it should be adjusted too, but my recommendation is to stick to 18 max.

    function _computeCRNew(
        uint256 _coll,
        uint256 _debt,
        uint256 _price,
        uint8 priceDecimals
    ) internal pure returns (uint256) {
        if (_debt > 0) {
+           if (priceDecimals < 18) {
+               uint8 decimalsDifference = 18 - priceDecimals;
+               _coll = _coll * (10 ** decimalsDifference);
+           }
            uint256 newCollRatio = (_coll * _price * 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;
        }
    }

Duplicate of #35