Cally does not support ERC20 tokens with built-in fee

[code423n4]

Lines of code

https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L174 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L200 https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L296

Vulnerability details

Impact

Contract Cally does not properly handle ERC20 tokens that charge fee on their transfers. Implementation of such a tokens does not transfer exact amount provided to transfer() but part of it is charged as a fee, burned or used in some other way. This leads to incorrect accounting and effectively to loss of funds.

Proof of Concept

• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L174
• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L200
• https://github.com/code-423n4/2022-05-cally/blob/1849f9ee12434038aa80753266ce6a2f2b082c59/contracts/src/Cally.sol#L296

Tools Used

Manual Review / VSCode

Recommended Mitigation Steps

It is recommended to add support for ERC20 tokens with built-in fees. Example of the implementation:

uint256 startingBalance = IERC20(token).balanceOf(address(this));
IERC20(token).safeTransferFrom(msg.sender, address(this), amount);
uint256 endingBalance = IERC20(token).balanceOf(address(this));

vault.tokenIdOrAmount = endingBalance - startingBalance;

[outdoteth]

reference issue: https://github.com/code-423n4/2022-05-cally-findings/issues/39