merlin - Fee on transfer tokens isn't compatible with LenderCommitmentGroup_Smart.sol
Closed this issue · 0 comments
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