kennedy1030 - Invalid check in `LenderCommitmentGroup_Smart.burnSharesToWithdrawEarnings()`.

[sherlock-admin2]

kennedy1030

high

Invalid check in LenderCommitmentGroup_Smart.burnSharesToWithdrawEarnings().

Summary

Without any other interruption, a malicious user can break the LenderCommitmentGroup_Smart by conducting a series of actions: deposit, borrow, liquidate, donation, and withdraw.

Vulnerability Detail

Let's consider the following scenario:

1. Bob deposits 100 wei to an empty LenderCommitmentGroup_Smart.
   • state: Bob's share = 100, totalPrincipalTokensCommitted = 100, totalSupply = 100.
2. Bob borrows 80 wei with no duration.
3. Bob liquidates his loan at the time _loanDefaultedTimestamp + 86400 + 5000.
   • state: tokenDifferenceFromLiquidations = -40.
4. Bob donates 60 wei to the LenderCommitmentGroup_Smart.
5. Bob withdraws 99 shares.
   • state: totalPrincipalTokensWithdrawn = 60, totalSupply = 1.

Finally, the exchange rate will be (100 - 40 - 60) / 1 = 0, leading to breaking the LenderCommitmentGroup_Smart.

After that, if someone deposits, he will receive no share and Bob can take all of that. This problem occurs because there is no balance check in withdrawing.

Impact

Without anyother's interruption, a malicious user can break the LenderCommitmentGroup_Smart.

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#L307-L322

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#L396-L415

Tool used

Manual Review

Recommendation

Deposit and withdrawal should be improved as follows.

    function addPrincipalToCommitmentGroup(
        uint256 _amount,
        address _sharesRecipient
    ) external returns (uint256 sharesAmount_) {
        //transfers the primary principal token from msg.sender into this contract escrow

        principalToken.transferFrom(msg.sender, address(this), _amount);

        sharesAmount_ = _valueOfUnderlying(_amount, sharesExchangeRate());
+       require(sharesAmount > 0); 

        totalPrincipalTokensCommitted += _amount;
        //principalTokensCommittedByLender[msg.sender] += _amount;

        //mint shares equal to _amount and give them to the shares recipient !!!
        poolSharesToken.mint(_sharesRecipient, sharesAmount_);
    }

    function burnSharesToWithdrawEarnings(
        uint256 _amountPoolSharesTokens,
        address _recipient
    ) external returns (uint256) {

        poolSharesToken.burn(msg.sender, _amountPoolSharesTokens);

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

+       int256 balanceOfPool = int256(totalPrincipalTokensCommitted) - int256(totalPrincipalTokensWithdrawn) 
+        + int256(totalInterestCollected)  + int256(tokenDifferenceFromLiquidations) 
+        - int256(totalPrincipalTokensLended) + int256(totalPrincipalTokensRepaid);

+       require(balanceOfPool > 0 && principalTokenValueToWithdraw > 0);
+       require(principalTokenValueToWithdraw <= uint256(balanceOfPool));          

        totalPrincipalTokensWithdrawn += principalTokenValueToWithdraw;

        principalToken.transfer(_recipient, principalTokenValueToWithdraw);

        return principalTokenValueToWithdraw;
    }

[nevillehuang]

Invalid, direct donation is not possible to influence shares exchange rate since pool value used to calculate rate does not include principle token balance for calculations to determine principalTokenValueToWithdraw.