Tadle

Summary

The _transfer function in the TokenManager.sol contract has a potential vulnerability when interacting with [Weird ERC20 tokens](https://github.com/d-xo/weird-erc20), specifically those that implement transfer fees, burning mechanisms, or other weird behaviors. This issue could cause the function to revert transactions unnecessarily, leading to potential disruptions in the intended operation of the contract and loss of funds.

Vulnerability Details

See TokenManager.sol::_transfer

https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/TokenManager.sol#L233-L262

The _transfer function checks the balance of the sender (_from) and receiver (_to) before and after the token transfer, expecting the balances to change exactly by the _amount specified. However, if the token implements non-standard behavior like deducting a fee on transfer or burning a portion of the tokens, these checks will fail, causing the function to revert unnecessarily.

For example, for a token with a 5% transfer fee, a transfer of 100 tokens would result in the receiver getting 95 tokens. The current implementation of _transfer expects the receiver to get the full 100 tokens, so it will incorrectly revert the transaction.

Impact

The erroneous balance checks will cause legitimate transactions to revert, potentially leading to disruptions in the system’s functionality. End-users might experience failed transactions without clear understanding or reason, leading to a loss of funds, and a loss of trust in the system.

Tools Used

Manual Review

Recommendations

Replace the custom _transfer implementation with OpenZeppelin’s safeTransfer and safeTransferFrom functions. These functions are battle-tested and handle edge cases across a wide range of ERC20 implementations.