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

rvierdiiev - Vault_Lyra.increaseCollateralAmount doesn't allow borrower to add collateral if total collateral is less than liquidation amount #41

Closed
github-actions bot opened this issue Dec 11, 2022 · 1 comment

Comments

@github-actions
Copy link

github-actions bot commented Dec 11, 2022

rvierdiiev

high

Vault_Lyra.increaseCollateralAmount doesn't allow borrower to add collateral if total collateral is less than liquidation amount

Summary

Vault_Lyra.increaseCollateralAmount doesn't allow borrower to add collateral if total collateral is less than liquidation amount. As a result borrower can't decrease his liquidatable amount.

Vulnerability Detail

Vault_Lyra.increaseCollateralAmount allows borrower to add more collateral to the vault.
https://github.com/sherlock-audit/2022-11-isomorph/blob/main/contracts/Isomorph/contracts/Vault_Lyra.sol#L155-L192

    function increaseCollateralAmount(
        address _collateralAddress,
        uint256 _colAmount
        ) external override whenNotPaused 
        {
        _collateralExists(_collateralAddress);
        require(collateralPosted[_collateralAddress][msg.sender] > 0, "No existing collateral!"); //feels like semantic overloading and also problematic for dust after a loan is 'closed'
        require(_colAmount > 0 , "Zero amount"); //Not strictly needed, prevents event spamming though
        //make sure virtual price is related to current time before fetching collateral details
        //slither-disable-next-line reentrancy-vulnerabilities-1
        _updateVirtualPrice(block.timestamp, _collateralAddress);
        IERC20 collateral = IERC20(_collateralAddress);
        require(collateral.balanceOf(msg.sender) >= _colAmount, "User lacks collateral amount");
        (   
            bytes32 currencyKey,
            ,
            uint256 liquidatableMargin,
            ,
            ,
            uint256 virtualPrice,
            
        ) = _getCollateral(_collateralAddress);
        //check for frozen or paused collateral
        _checkIfCollateralIsActive(currencyKey);
        //debatable check begins here 
        uint256 totalCollat = collateralPosted[_collateralAddress][msg.sender] + _colAmount;
        uint256 colInUSD = priceCollateralToUSD(currencyKey, totalCollat);
        uint256 USDborrowed = (isoUSDLoanAndInterest[_collateralAddress][msg.sender] * virtualPrice) / LOAN_SCALE;
        uint256 borrowMargin = (USDborrowed * liquidatableMargin) / LOAN_SCALE;
        require(colInUSD >= borrowMargin, "Liquidation margin not met!");
        //debatable check ends here
        //update mapping with new collateral amount
        collateralPosted[_collateralAddress][msg.sender] = collateralPosted[_collateralAddress][msg.sender] + _colAmount;
        emit IncreaseCollateral(msg.sender, currencyKey, _colAmount);
        //Now all effects are handled, transfer the collateral so we follow CEI pattern
        _increaseCollateral(collateral, _colAmount);
        
    }

This function checks if the total collateral amount is less than liquidatable amount.
require(colInUSD >= borrowMargin, "Liquidation margin not met!");
If yes, then operation is successful, if no then function reverts.

So when user's loan is liquidatable he can't increase collateral for some amount to make liquidatable amount smaller.

Example.
1.User has loan for 100$
2.Loan becomes liquidatable and user has collateral amount 80$ worth.
3.User wants to deposit more 10$ of collateral to make liquidatable amount smaller, but he can't.
4.Someone liquidates liquidatable amount of user, he lost some funds.

Impact

User can't make liquidatable amount smaller.

Code Snippet

https://github.com/sherlock-audit/2022-11-isomorph/blob/main/contracts/Isomorph/contracts/Vault_Lyra.sol#L155-L192

Tool used

Manual Review

Recommendation

Allow user to deposit collateral even if total collateral amount is less than liquidatable border.

Duplicate of #229

@github-actions github-actions bot added the High label Dec 11, 2022
@kree-dotcom
Copy link

Sponsor confirmed, Will Fix, Disagree with severity. Intended design, mentioned in documentation, so likely a low issue? In documentation we mention the section commented "//debatable check begins here" shown in the snippet above but we have decided the auditors comment makes the system fairer and so we will alter the design.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

3 participants