Tokens with fee on transfer are not supported

[code423n4]

Handle

WatchPug

Vulnerability details

There are ERC20 tokens that charge fee for every transfer() or transferFrom().

In the current implementation, BorrowerOperations.sol#_transferCollateralsIntoActivePool() assumes that the received amount is the same as the transfer amount, and uses it as collateral.

https://github.com/code-423n4/2021-12-yetifinance/blob/5f5bf61209b722ba568623d8446111b1ea5cb61c/packages/contracts/contracts/BorrowerOperations.sol#L731-L748

    function _transferCollateralsIntoActivePool(
        address _from,
        address[] memory _colls,
        uint256[] memory _amounts
    ) internal returns (bool) {
        uint256 len = _amounts.length;
        for (uint256 i = 0; i < len; i++) {
            address collAddress = _colls[i];
            uint256 amount = _amounts[i];
            IERC20 coll = IERC20(collAddress);

            bool transferredToActivePool = coll.transferFrom(_from, address(activePool), amount);
            if (!transferredToActivePool) {
                return false;
            }
        }
        return true;
    }

Recommended

• Compare the before and after balanceOf to get the actual transferred amount;
• Or, not to whitelist any tokens with fee on transfer.

[kingyetifinance]

@LilYeti: We will not be taking any collaterals with transfer fees / have a strong vetting process for whitelisting. Severity 0 due to this.

[alcueca]

There is no mention in the accompanying documentation about fee-on-transfer tokens not being supported. Low severity sustained.