The Standard

Summary

Fee on transfer tokens are not properly accounted for in some places in the protocol. This might result in unexpected reverts due to Insufficient Balance errors and other accounting issues.

Vulnerability Details

There are two places I found that do not consider that the input token might be a fee-on-transfer one.

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/SmartVaultV3.sol#L215

https://github.com/Cyfrin/2023-12-the-standard/blob/main/contracts/LiquidationPool.sol#L219-L220

Impact

In the case of SmartVaultV3::swap(), it will cause wrong fee distribution to the protocol or even the swap to revert altogether due to the wrong calculation of amountOutMinimum when there is not enough balance within the vault. (Look at SmartVaultV3::executeERC20SwapAndFee())

In the case of LiquidationPool::distributeAssets() (when called by by a user, not LiquidationPoolManager), it will cause wrong accounting and will eventually revert when someone tries to call claimRewards() because the contract won't have the same balance it had calculated in distributeAssets().

This will also fix issues with tokens like cUSDCV3. That one isn't technically a fee on transfer but it has a quirk where if the transfer amount is type(uint256).max and the user doesn't have it, it won't revert but simply send the user's current total balance.

Tools Used

Manual Analysis

Recommendations

Check the balances received before performing calculations with them.