20,000 USDC
View results
Submission Details
Severity: high
Valid

Fee on transfer tokens will not behave as expected

Summary

Integrating fee-on-transfer tokens would completely break beedle.fi's internal accounting

Vulnerability Details

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

/// @notice deposit tokens to stake
/// @param _amount the amount to deposit
function deposit(uint _amount) external {
TKN.transferFrom(msg.sender, address(this), _amount);
updateFor(msg.sender);
balances[msg.sender] += _amount;
}

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:

/// @notice withdraw tokens from stake
/// @param _amount the amount to withdraw
function withdraw(uint _amount) external {
updateFor(msg.sender);
balances[msg.sender] -= _amount;
TKN.transfer(msg.sender, _amount);
}

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.

Impact

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.

Tool Used

Manual Audit

Recommendation

Check the balances before and after any transfer with fees to ensure accurate accounting.

Additional Note

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

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.