use safetransfer instead of transfer

Summary

tokens not compliant with the ERC20 specification could return false from the transfer function call to indicate the transfer fails, while the calling contract would not notice the failure if the return value is not checked. Checking the return value is a requirement, as written in the EIP-20 specification:

Vulnerability Detail

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

we cannot know whether the token is transferred or not. we shou;ld check the return value to know whether token is transfer or not.

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#L412

Tool used

Manual Review

Recommendation

Use the SafeERC20 library implementation from OpenZeppelin and call safeTransfer or safeTransferFrom when transferring ERC20 tokens.

Duplicate of #50

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

bareli - use safetransfer instead of transfer #221