pkqs90 - `LenderCommitmentGroup_Smart.sol#sharesExchangeRateInverse()` may divide by zero

[sherlock-admin3]

pkqs90

medium

LenderCommitmentGroup_Smart.sol#sharesExchangeRateInverse() may divide by zero

Summary

In LenderCommitmentGroup_Smart.sol#sharesExchangeRateInverse(), it divides sharesExchangeRate() which may be zero. This will block users from calling burnSharesToWithdrawEarnings() to burn shares.

Vulnerability Detail

getPoolTotalEstimatedValue() may return zero if tokenDifferenceFromLiquidations is too small due to liquidation. This will cause sharesExchangeRate() to return 0, and cause a divide by zero in sharesExchangeRateInverse(). This will cause users to not be able to call burnSharesToWithdrawEarnings().

Note that in this case, if the function doesn't revert, the withdrawn tokens may still be zero. However, this is still an issue, because:

1. This is a core function that reverts unexpectedly.
2. Users may use this function to simply burn shares, and doesn't care about withdrawn tokens.

    function sharesExchangeRate() public view virtual returns (uint256 rate_) {
        

        uint256 poolTotalEstimatedValue = getPoolTotalEstimatedValue();

        if (poolSharesToken.totalSupply() == 0) {
            return EXCHANGE_RATE_EXPANSION_FACTOR; // 1 to 1 for first swap
        }

>       rate_ =
>           (poolTotalEstimatedValue  *
>               EXCHANGE_RATE_EXPANSION_FACTOR) /
>           poolSharesToken.totalSupply();
    }

    function sharesExchangeRateInverse()
        public
        view
        virtual
        returns (uint256 rate_)
    {
        return
            (EXCHANGE_RATE_EXPANSION_FACTOR * EXCHANGE_RATE_EXPANSION_FACTOR) /
>           sharesExchangeRate();
    }

    function getPoolTotalEstimatedValue()
        public
        view
        returns (uint256 poolTotalEstimatedValue_)
    {
       
         int256 poolTotalEstimatedValueSigned = int256(totalPrincipalTokensCommitted) 
         + int256(totalInterestCollected)  + int256(tokenDifferenceFromLiquidations) 
         - int256(totalPrincipalTokensWithdrawn);

        //if the poolTotalEstimatedValue_ is less than 0, we treat it as 0.  
        poolTotalEstimatedValue_ = poolTotalEstimatedValueSigned > int256(0)
            ? uint256(poolTotalEstimatedValueSigned)
            : 0;
    }

    function burnSharesToWithdrawEarnings(
        uint256 _amountPoolSharesTokens,
        address _recipient
    ) external returns (uint256) {
              
        poolSharesToken.burn(msg.sender, _amountPoolSharesTokens);

>       uint256 principalTokenValueToWithdraw = _valueOfUnderlying(
            _amountPoolSharesTokens,
            sharesExchangeRateInverse()
        );

        totalPrincipalTokensWithdrawn += principalTokenValueToWithdraw;

        principalToken.transfer(_recipient, principalTokenValueToWithdraw);

        return principalTokenValueToWithdraw;
    }

Impact

Users can't to call burnSharesToWithdrawEarnings(), because it will revert on dividing by zero.

Code Snippet

• https://github.com/sherlock-audit/2024-04-teller-finance/blob/main/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L277-L286

Tool used

Manual review

Recommendation

If sharesExchangeRate() is zero, simply also return 0 for sharesExchangeRateInverse().

Duplicate of #64