Integrating fee-on-transfer tokens would completely break beedle.fi's internal accounting
The current implementation of the beedle.fi protocol doesn't work with fee-on-transfer underlying tokens, emphasis on both the staking.sol and lender.sol contracts
When a fee is charged on a transfer of tokens in Solidity, it is important to check the balance of the sender's account before and after the transfer to ensure that the fee has been correctly deducted from the sender's balance.
Take a look at Staking.sol#L36-L42
As seen no check is performed to really check if the amount of tokens received are what was sent note that since this check is not performed, it can result in an accounting error where the sender's balance is overstated, leading to potential issues with record keeping, reporting, and reconciliation.
For example the withdraw()) function:
When this is called with the "amount" that was deposited, the contract would try to transfer the "amount" and as such would lead to taking part of other users deposit.
Break of internal accounting, in the case of the deposit()
function of Staking.sol
, the contract directly registers the amount provided and not the one received.
Manual Audit
Check the balances before and after any transfer with fees to ensure accurate accounting.
Would be key to note that this issue has multiple instances within the protocol (over 20 instances of transfer/trasferFrom in Lender.sol), but for brevity reasons this report only focused on the instance in Beedle.sol
contract, advisably fixes should be made into all instances, lastly do note that there are some tokens who do not collect fees with transfers now, but this could change later on and this should be taken into account while this is being mitigated.
Sure, here's the reformatted audit report for the contract Staking.sol
The contest is live. Earn rewards by submitting a finding.
This is your time to appeal against judgements on your submissions.
Appeals are being carefully reviewed by our judges.