Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Fee on transfer tokens can cause lender tokens to get stuck in contract.

33audits

Summary

Fee on transfer tokens take a tax when they are submitted

Vulnerability Details

Some ERC20 tokens, such as USDT, allow for charging a fee any time transfer() or transferFrom() is called. If a contract does not allow for amounts to change after transfers, subsequent transfer operations based on the original amount will revert() due to the contract having an insufficient balance. However, a malicious user could also take advantage of this to steal funds from the pool.

Let's take a look at the addPool() and removePool() functions.

/// @notice remove from the pool balance
/// can only be called by the pool lender
/// @param poolId the id of the pool to remove from
/// @param amount the amount to remove
function removeFromPool(bytes32 poolId, uint256 amount) external {
if (pools[poolId].lender != msg.sender) revert Unauthorized();
if (amount == 0) revert PoolConfig();
_updatePoolBalance(poolId, pools[poolId].poolBalance - amount);
// transfer the loan tokens from the contract to the lender
IERC20(pools[poolId].loanToken).transfer(msg.sender, amount);
}
​
/// @notice add to the pool balance
/// can only be called by the pool lender
/// @param poolId the id of the pool to add to
/// @param amount the amount to add
function addToPool(bytes32 poolId, uint256 amount) external {
if (pools[poolId].lender != msg.sender) revert Unauthorized();
if (amount == 0) revert PoolConfig();
​
_updatePoolBalance(poolId, pools[poolId].poolBalance + amount);
// transfer the loan tokens from the lender to the contract
IERC20(pools[poolId].loanToken).transferFrom(
msg.sender,
address(this),
amount
);
}
​

Impact

addToPool() takes in an amount that is set by the users. removeFromPool() does the same, however when attempting to remove a pool using fee on transfer tokens the function would revert due to insufficient balance leaving the users funds stuck in the pool. Consider the example below.

Step to Reproduce

  1. Alice sends 100 of FEE tokens to the contract when calling addToPool().

  2. FEE token contract takes 10% of the tokens (10 FEE).

  3. 90 FEE tokens actually get deposited in contract.

  4. _updatePoolBalance(poolId, pools[poolId].poolBalance + amount); will equal 100.

  5. Attacker then sends 100 FEE tokens to the contract

  6. The contract now has 180 FEE tokens but each user has an accounting of 100 FEE.

  7. The attacker then tries to redeem his collateral for the full amount 100 FEE tokens.

  8. The contract will transfer 100 FEE tokens to Bob taking 10 of Alices tokens with him.

  9. Bob can then deposit back into the pool and repeat this until he drains all of Alice's funds.

  10. When Alice attempts to withdraw the transaction will revert due to insufficient funds.

Tools used

Manual Review

Recommended Steps

Measure the contract balance before and after the call to transfer()/transferFrom() and use the difference between the two as the amount, rather than the amount stated

function addToPool(bytes32 poolId, uint256 amount) external {
if (pools[poolId].lender != msg.sender) revert Unauthorized();
if (amount == 0) revert PoolConfig();
uint256 balanceBefore = IERC20(pools[poolId].loanToken).balanceOf(address(this))
​
IERC20(pools[poolId].loanToken).transferFrom(
msg.sender,
address(this),
amount
);
uint256 balanceAfter = IERC20(pools[poolId].loanToken).balanceOf(address(this))
uint256 _amount = balanceBefore - balanceAfter
_updatePoolBalance(poolId, pools[poolId].poolBalance + _amount);
// transfer the loan tokens from the lender to the contract
}

Note this implementation is vulnerable to reentrancy so use a nonreentrant guard in on this function.

Support

FAQs

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