sherlock-audit / 2023-05-perennial-judging

12 stars 9 forks source link

levi - `collateralInvariant` is restrictive on deposits #196

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

levi

medium

collateralInvariant is restrictive on deposits

Summary

collateralInvariant is restrictive on deposits

Vulnerability Detail

When depositing, the collateralInvariant modifier is checked first

    function depositTo(address account, IProduct product, UFixed18 amount)
    external
    nonReentrant
    notPaused
    notZeroAddress(account)
    isProduct(product)
    collateralInvariant(account, product)
    {
        _products[product].creditAccount(account, amount);
        token.pull(msg.sender, amount);

        emit Deposit(account, product, amount);
    }
    /// @dev Ensure that the account is either empty or above the collateral minimum
    modifier collateralInvariant(address account, IProduct product) {
        _;

        UFixed18 accountCollateral = collateral(account, product);
        if (!accountCollateral.isZero() && accountCollateral.lt(controller().minCollateral()))
            revert CollateralUnderLimitError();
    }

This is restrictive as it prevents deposits if an account goes below minCollateral() which could occur as collateral gets devalued. This prevents a user from topping up their collateral.

Impact

Users are prevented from improving their position in case of collateral devaluation.

Code Snippet

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/collateral/Collateral.sol#L51-L58

https://github.com/sherlock-audit/2023-05-perennial/blob/main/perennial-mono/packages/perennial/contracts/collateral/Collateral.sol#L274-L280

Tool used

Manual Review

Recommendation

Consider relaxing the deposit requirements to enable users to improve their collateral position