Users might be liquidated if the `vaultSafeCollateralRatio[pool]` is updated to a higher value.

[code423n4]

Lines of code

https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraEUSDVaultBase.sol#L291-L293 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol#L225-L228 https://github.com/code-423n4/2023-06-lybra/blob/7b73ef2fbb542b569e182d9abf79be643ca883ee/contracts/lybra/configuration/LybraConfigurator.sol#L198-L206

Vulnerability details

Details

• In LybraPeUSDVaultBase.sol & LybraEUSDVaultBase.sol : _checkHealth function uses configurator.getSafeCollateralRatio(address(this)) to check if the user position is healthy or not (eligible for liquidation).

• The returned value of getSafeCollateralRatio(vaultAddress) if the (vaultSafeCollateralRatio[pool]) is not set yet is 160 * 1e18.

• The account with TIMELOCK privilage in the LybraConfigurator.sol contract can set the vaultSafeCollateralRatio[pool] to a value greater than 160 * 1e18 for the valultType=0 & to a value greater than vaultBadCollateralRatio[pool] + 1e19 for the valultType=1, and there's no maximum limit to check on the new value.

Impact

• Setting the safeCollateralRatio to higher values will make users positions unhealthy and some of these positions might be eligible to liquidation.

Proof of Concept

• In LybraConfigurator.sol:

File: 2023-06-lybra/contracts/lybra/configuration/LybraConfigurator.sol
Line 198-206:
 function setSafeCollateralRatio(address pool, uint256 newRatio) external checkRole(TIMELOCK) {
        if(IVault(pool).vaultType() == 0) {
            require(newRatio >= 160 * 1e18, "eUSD vault safe collateralRatio should more than 160%");
        } else {
            require(newRatio >= vaultBadCollateralRatio[pool] + 1e19, "PeUSD vault safe collateralRatio should more than bad collateralRatio");
        }
        vaultSafeCollateralRatio[pool] = newRatio;
        emit SafeCollateralRatioChanged(pool, newRatio);
    }

• In LybraEUSDVaultBase.sol:

File: 2023-06-lybra/contracts/lybra/pools/base/LybraEUSDVaultBase.sol
Line 291-293:
    function _checkHealth(address _user, uint256 _assetPrice) internal view {
        if (((depositedAsset[_user] * _assetPrice * 100) / borrowed[_user]) < configurator.getSafeCollateralRatio(address(this))) revert("collateralRatio is Below safeCollateralRatio");
    }

• In LybraPeUSDVaultBase.sol:

File: 2023-06-lybra/contracts/lybra/pools/base/LybraPeUSDVaultBase.sol
Line 225-228:
    function _checkHealth(address user, uint256 price) internal view {
        if (((depositedAsset[user] * price * 100) / getBorrowedOf(user)) < configurator.getSafeCollateralRatio(address(this)))
            revert("collateralRatio is Below safeCollateralRatio");
    }

Tools Used

Manual Testing.

Recommended Mitigation Steps

A recommended mitigation step is to set a struct to track each position (debt) made by the user where the current safeCollateralRatio at the time of the borrow is recorded, and another array to track users positions, so when checking for the health of the user,a weighted average of the health of all positions made by a user is used to checkHealth.

Assessed type

Other

[c4-pre-sort]

JeffCX marked the issue as primary issue

[LybraFinance]

Modifying the liquidation collateral ratio requires approval through DAO governance. Typically, users will have sufficient time to provide additional collateral.

[c4-sponsor]

LybraFinance marked the issue as sponsor acknowledged

[c4-judge]

0xean changed the severity to QA (Quality Assurance)