Fee on transfer ERC20s cause DOS on essential functions
Vulnerability Details 🔍 && Impact 📈
Protocol claims to support any ERC20 token yet some ERC20 have the fee on token transfer functioanlity which leads to a DOS on critical functions.
Fee on token transfer means that if from sends amount to to, to will receive amount - fee.
Thus if using this tokens, in _transfer() on TokenManager.sol the following checks will revert:
This causes a DOS of _transfer() when used with fee on transfer tokens. The impact is high as _transfer() is a function that gets called in critical parts of the protool like in the withdraw() or tillIn(), functions. Functions used for moving value around in and out of the system.
Also, some tokens might have the ability of turning on and off the fees, if they turn on the fees after people have made maker-taker operations in Tadle, the collateral will be stuck as it won't be able to be withdrawn.
Recommendations 🎯
As collateral tokens have to be whitelisted by the team (see here), add a special flag that indicates if some whitelisted collateral includes fee on transfer. In those cases you can use a special check that allows for a little bit of difference in the prev and post balance checks.
Lead Judging Commences
finding-TokenManager-FOT-Rebasing
Valid medium, there are disruptions to the ability to take market actions. The following functions will be disrupted without the possibiliy of reaching settlement, since the respective offers cannot be created/listed regardless of mode when transferring collateral token required to the CapitalPool contract or when refunding token from user to capital pool during relisting. So withdrawal is not an issue - `createOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L96-L102) - `listOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L355-L362) - `relistOffer()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L515-L521) - `createTaker()` - reverts [here](https://github.com/Cyfrin/2024-08-tadle/blob/04fd8634701697184a3f3a5558b41c109866e5f8/src/core/PreMarkets.sol#L831-L836) I believe medium severity is appropriate although the likelihood is high and impact is medium (only some level of disruption i.e. FOT tokens not supported and no funds at risk)
Prize pool breakdown
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.