sherlock-audit / 2023-03-taurus-judging

4 stars 0 forks source link

peanuts - User can be liquidated immediately after taking maximal debt #176

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

peanuts

medium

User can be liquidated immediately after taking maximal debt

Summary

Since there’s no gap between the maximal collaterization ratio and the liquidation ratio, user positions may be liquidated as soon as maximal debt is taken, without leaving room for collateral price fluctuation. Users have no chance to add more collateral or reduce debt before being liquidated. This may eventually create more uncovered and bad debt for the protocol.

Vulnerability Detail

When adding a position, the user calls modifyPosition() which sets the amount of collateral and debt he wants to take. Since the debt is increased, the _checkAccountHealth(_account) clause is called.

    function _modifyPosition(
        address _account,
        uint256 _collateralDelta,
        uint256 _debtDelta,
        bool _increaseCollateral,
        bool _increaseDebt
    ) internal virtual {
        bool mustCheckHealth; // False until an action is taken which can reduce account health

        // Handle debt first, since TAU has no reentrancy concerns.
       // @-- audit if debt is taken, mustCheckHealth becomes true
        if (_debtDelta != 0) {
            if (_increaseDebt) {
                // Borrow TAU from the vault
                userDetails[_account].debt += _debtDelta;
                mustCheckHealth = true;
                TAU(tau).mint(_account, _debtDelta);

                emit Borrow(_account, _debtDelta);
            } else {
                // Repay TAU debt
                uint256 currentDebt = userDetails[_account].debt;
                if (_debtDelta > currentDebt) _debtDelta = currentDebt;
                userDetails[_account].debt = currentDebt - _debtDelta;
                // Burn Tau used to repay debt
                TAU(tau).burnFrom(_account, _debtDelta);

                emit Repay(_account, _debtDelta);
            }
        }

        if (_collateralDelta != 0) {
            if (_increaseCollateral) {
                // Deposit collateral
                userDetails[_account].collateral += _collateralDelta;
                IERC20(collateralToken).safeTransferFrom(msg.sender, address(this), _collateralDelta);

                emit Deposit(_account, _collateralDelta);
            } else {
                // Withdraw collateral
                uint256 currentCollateral = userDetails[_account].collateral;
                if (_collateralDelta > currentCollateral) revert insufficientCollateral();
                userDetails[_account].collateral = currentCollateral - _collateralDelta;
                mustCheckHealth = true;
                IERC20(collateralToken).safeTransfer(msg.sender, _collateralDelta);

                emit Withdraw(_account, _collateralDelta);
            }
        }

// @--audit calls `_checkAccountHealth()`
        if (mustCheckHealth) {
            _checkAccountHealth(_account);
        }
    }

The checkAccountHealth then check if ratio is above the MIN_COL_RATIO, which is 120% or 1.2e18

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

        return (ratio > MIN_COL_RATIO);
    }

When it is time to liquidate, the Liquidator calls liquidate() and it checks whether the account is liquitable. liquidate() calls _calcLiquidation() which calls _calcLiquidationDiscount() and checks that the account Health is lower than the MIN_COL_RATIO. If a user's account is below MIN_COL_RATIO, which is 120%, he can get liquidated.

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

The user can accrue maximum debt and get liquidated almost immediately.

Impact

The code incentivizes MEV bots to liquidate max debt positions, in almost the same block.

Code Snippet

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

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

Tool used

Manual Review

Recommendation

Recommend adding a liquidation ratio that is smaller than the maximal collaterization ratio so that position can only be liquidated after reaching liquidation ratio (i.e 115% or 1.5e18) to allow for price fluctuation.

For example, AAVE has a Max LTV ratio of 82.5% (or a collaterization ratio of ~121%) and a Liquidation LTV of 86% (or a liquidation ratio of ~116%).

Another token, LINK, on the AAVE platform has a max LTV of 70% (~142% collaterization ratio) ( and liquidation threshold of 83% (~120% liquidation ratio)

Sierraescape commented 1 year ago

This seems pretty similar to #145. The solution is the same in both cases--we have a higher minimum collateral ratio set in the front end. So long as users are aware of the risks, I don't think that allowing them to be more degen is a vulnerability per se.