merlin - Fee on transfer tokens isn't compatible with LenderCommitmentGroup_Smart.sol

[sherlock-admin2]

merlin

medium

Fee on transfer tokens isn't compatible with LenderCommitmentGroup_Smart.sol

Summary

From the previous audit, the protocol intended to work with FEE-ON-TRANSFER: any, and there was also a valid issues from previous audit regarding fee-on-transfer compliance. There are also comments in TellerV2.sol confirming that the protocol interacts with fee-on-transfer tokens: contracts/contracts/TellerV2.sol#L934

//used for fee-on-send tokens
uint256 paymentAmountReceived = balanceAfter - balanceBefore;

Vulnerability Detail

The addPrincipalToCommitmentGroup function mints shares with the amount passed as an argument, without checking the actual amount of tokens received.

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

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

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

Impact

Fee on transfer tokens isn't compatible with LenderCommitmentGroup_Smart.sol

Code Snippet

contracts/contracts/TellerV2.sol#L934 contracts/contracts/LenderCommitmentForwarder/extensions/LenderCommitmentGroup/LenderCommitmentGroup_Smart.sol#L307

Tool used

Manual Review

Recommendation

Calculate the amount of tokens that will be received by the LenderCommitmentGroup_Smart contract by subtracting balanceOf before and after, instead of assuming the amount of tokens that will be received.

Duplicate of #122