sherlock-audit / 2023-04-splits-judging

4 stars 1 forks source link

ss3434 - Some ERC20 tokens deduct a fee on transfer #59

Closed sherlock-admin closed 1 year ago

sherlock-admin commented 1 year ago

ss3434

medium

Some ERC20 tokens deduct a fee on transfer

Summary

Some ERC20 token implementations have a fee that is charged on each token transfer. This means that the transferred amount isn't exactly what the receiver will get. The protocol currently uses any ERC20 tokens:

ERC20: any non-rebasing

Vulnerability Detail

See Summary

Impact

The transferred amount isn't exactly what the receiver will get.

Code Snippet

https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-utils/src/TokenUtils.sol#L26 https://github.com/sherlock-audit/2023-04-splits/blob/main/splits-swapper/src/SwapperImpl.sol#L278

Tool used

Manual Review

Recommendation

Improve support for fee on transfer type of ERC20. When pulling funds from the user using safeTransferFrom and safeTransfer the usual approach is to compare balances pre/post transfer, like so: This is example(chek balance) :

uint256 balanceBefore = IERC20(token).balanceOf(address(this));
IERC20(token).transferFrom(msg.sender, address(this), amount);
uint256 transferred = IERC20(token).balanceOf(address(this)) - balanceBefore;