Some collateral will be locked in the contract

Summary

Some tokens will be locked in the LenderCommitmentGroup_Smart contract forever if used as principal token.

Vulnerability Detail

Some ERC20 tokens, such as BNB, do not revert but return a bool value if the transfer() and transferFrom() function calls fail. Therefore, the transaction may not be reverted and funds may be lost.

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

403:    poolSharesToken.burn(msg.sender, _amountPoolSharesTokens);

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

        totalPrincipalTokensWithdrawn += principalTokenValueToWithdraw;

412:    principalToken.transfer(_recipient, principalTokenValueToWithdraw);

        return principalTokenValueToWithdraw;
    }

Even if token transmission fails in #L412, the burnSharesToWithdrawEarnings() function is not reverted. Therefore, users may lose their poolSharesToken.

Impact

If the token send fails, it will cause a lot of serious problems. For example, if a token such as BNB is used as the principal token, the user may lose the poolShareToken.

Code Snippet

Tool used

Manual Review

Recommendation

Use OpenZeppelin's safeTransfer(), which is able to handle the missing return value

Duplicate of #50

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

blockchain555 - Some collateral will be locked in the contract #249