Panic Reverts in `sharesExchangeRateInverse` could break the implementation of `burnSharesToWithdrawEarnings`

Low/Info issue submitted by Bauchibred

Summary

The sharesExchangeRateInverse function in the LenderCommitmentGroup_Smart.sol contract may lead to a panic revert when the sharesExchangeRate function returns a rate of 0. This could potentially block all queries to the burnSharesToWithdrawEarnings function.

Vulnerability Detail

In the sharesExchangeRateInverse function, the line return (EXCHANGE_RATE_EXPANSION_FACTOR * EXCHANGE_RATE_EXPANSION_FACTOR) / sharesExchangeRate(); performs a division operation by calling the sharesExchangeRate function. If the sharesExchangeRate function returns a rate of 0, the division operation will result in a panic revert due to division by zero.

Note that the sharesExchangeRate can return 0 when the poolTotalEstimatedValue is negative as shown in the snippets below: https://github.com/sherlock-audit/2024-04-teller-finance/blob/defe55469a2576735af67483acf31d623e13592d/teller-protocol-v2-audit-2024/packages/contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L263-L303

    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() {
        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;
    }

Impact

When the sharesExchangeRate function returns a rate of 0, the sharesExchangeRateInverse function will panic revert, leading to a potential panic state in the contract.

This would obviously have serious consequences, as it blocks all queries to the burnSharesToWithdrawEarnings function since it directly queries sharesExchangeRateInverse() to get the rate to pass to _valueOfUnderlying , preventing users from withdrawing their earnings and breaking core functionality.

Tool used

Manual Review

Recommendation

To avoid potential panic reverts and ensure uninterrupted functionality, consider implementing appropriate checks in the sharesExchangeRateInverse function to handle the scenario where the sharesExchangeRate function returns a rate of 0. This could involve adding conditional statements to verify the validity of the rate before performing the division operation. Additionally, ensure that proper error handling mechanisms are in place to gracefully handle such scenarios and provide informative error messages to users.

sherlock-audit / 2024-04-teller-finance-judging