fee on transfer token will cause accounting error

Summary

In drawDebt, the pledgedCollateral and t0Debt is updated with user specified input before the transfer. However, with fee on transfer tokens, the received and sent tokens will be lesser after the transfer. This will result in user receiving lesser than recorded debt and also the protocol will have lesser collateral than recorded collateral. This will become problematic when repaying debt as protocol might not have enough collateral to send back to the last few borrowers.

Vulnerability Detail

Notice how the accounting is done before the transfer.

            poolBalances.pledgedCollateral += collateralToPledge_;

            // move collateral from sender to pool
            _transferCollateralFrom(msg.sender, collateralToPledge_);

            poolBalances.t0Debt += result.t0DebtChange;

            // move borrowed amount from pool to sender
            _transferQuoteToken(msg.sender, amountToBorrow_);

Applies to ERC20Pool collateral and quote tokens and ERC721Pool quote tokens.

Impact

1) Collateral recorded will always be more than what the pool has, preventing final few borrowers or liquidity providers from being able to withdraw their collateral. 2) Debt recorded is more than what user receives.

Code Snippet

ERC20Pool.sol#L160-L163 ERC20Pool.sol#L168-L171 ERC721Pool.sol#L177-L180

Tool used

Manual Review

Recommendation

Recommend using the common pattern that checks before and after balance. Do for all transfers in the protocol.

uint256 balanceBefore = IERC20(_getArgAddress(COLLATERAL_ADDRESS)).balanceOf(address(this));
_transferCollateralFrom(msg.sender, collateralToPledge_);
uint256 balanceAfter = IERC20(_getArgAddress(COLLATERAL_ADDRESS)).balanceOf(address(this));
uint256 tokenAmount = balanceAfter - balanceBefore;
poolBalances.pledgedCollateral += tokenAmount;

sherlock-audit / 2023-01-ajna-judging

koxuan - fee on transfer token will cause accounting error #177