[MEDIUM-02] - Fee-on-transfer tokens are incorrectly handled in some situations

[ethereumdegen]

From previous audits, it seems the codebase should support Fee-on-transfer tokens. However, there are some situations where fees when transferring are not considered. A good example are the transfers performed in CollateralManager's deployAndDeposit. When a bid submission with collateral is created, _commitCollateral will directly store the amounts specified in _collateralInfo:

// File: CollateralManager.sol

function _commitCollateral( uint256 _bidId, Collateral memory _collateralInfo ) internal virtual { CollateralInfo storage collateral = _bidCollaterals[_bidId];

    require(
        !collateral.collateralAddresses.contains(
            _collateralInfo._collateralAddress
        ),
        "Cannot commit multiple collateral with the same address"
    );
    require(
        _collateralInfo._collateralType != CollateralType.ERC721 ||
            _collateralInfo._amount == 1,
        "ERC721 collateral must have amount of 1"
    );

    collateral.collateralAddresses.add(_collateralInfo._collateralAddress);
    collateral.collateralInfo[
        _collateralInfo._collateralAddress
    ] = _collateralInfo;
    emit CollateralCommitted(
        _bidId,
        _collateralInfo._collateralType,
        _collateralInfo._collateralAddress,
        _collateralInfo._amount,
        _collateralInfo._tokenId
    );
}

Then, when _deposit is triggered to transfer the corresponding collateral to the escrow, the amount to transfer will directly be collateralInfo._amount:

// File: CollateralManager.sol

function _deposit(uint256 _bidId, Collateral memory collateralInfo) internal virtual { require(collateralInfo._amount > 0, "Collateral not validated"); (address escrowAddress, address borrower) = _deployEscrow(_bidId); ICollateralEscrowV1 collateralEscrow = ICollateralEscrowV1( escrowAddress ); // Pull collateral from borrower & deposit into escrow if (collateralInfo._collateralType == CollateralType.ERC20) { IERC20Upgradeable(collateralInfo._collateralAddress).transferFrom( borrower, address(this), collateralInfo._amount ); IERC20Upgradeable(collateralInfo._collateralAddress).approve( escrowAddress, collateralInfo._amount ); collateralEscrow.depositAsset( CollateralType.ERC20, collateralInfo._collateralAddress, collateralInfo._amount, 0 );

        ...

As we can see, tokens will be first transferred from the borrower to the CollateralManager. If tokens include a fee-on-transfer, the received amount will actually be smaller than collateralInfo._amount. This will make the subsequent collateralEscrow.depositAsset call fail, as it tries to deposit exactly collateralInfo._amount into the escrow.

Recommendation Ensure that each transfer performed in the protocol accounts for fees on transfer. The pattern to follow is similar to the logic performed in the catch statement from _sendOrEscrowFunds:

// File: TellerV2.sol

function _sendOrEscrowFunds(uint256 _bidId, Payment memory _payment) internal { Bid storage bid = bids[_bidId]; address lender = getLoanLender(_bidId);

    uint256 _paymentAmount = _payment.principal + _payment.interest;

    try 

        bid.loanDetails.lendingToken.transferFrom{ gas: 100000 }(
            _msgSenderForMarket(bid.marketplaceId),
            lender,
            _paymentAmount
        )
    {} catch {
        address sender = _msgSenderForMarket(bid.marketplaceId);

        uint256 balanceBefore = bid.loanDetails.lendingToken.balanceOf(
            address(this)
        ); 

        //if unable, pay to escrow
        bid.loanDetails.lendingToken.safeTransferFrom(
            sender,
            address(this),
            _paymentAmount
        );

        uint256 balanceAfter = bid.loanDetails.lendingToken.balanceOf(
            address(this)
        );

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

Always perform the following steps:

Query the initial balance of the contract Perform the transfer Query the current balance after the transfer. The actual amount transferred will be currentBalance - initialBalance.

[ethereumdegen]

we are going to disallow fee-on-transfer tokens across the protocol #65