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

Re-entrancy issues in multiple functions

sonny2k

Summary

Some token standards, such as ERC777, allow a callback to the source of the funds (the from address) before the balances are updated in transferFrom(). This callback could be used to re-enter the function and inflate the amount.

Vulnerability Details

addtoPool() function and seizeLoan() function do the transfer of tokens before updating the state, which can lead to exploits if the token is a token that gives control to the sender, like ERC777 tokens.

addtoPool() increases the poolBalance of the pool before the transfering of loanToken from the lender of that pool. However, since there is no re-entrancy guard on this function, there is a risk of re-entrancy attack to increase the balance more without providing loanToken, which can be withdrawn later by the lender of the pool.

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
);
}

seizeLoan() transferring the collateralToken to the lender before updating the outstandingLoans of that pool and delete the loan. However, since there is no re-entrancy guard on this function, there is a risk of re-entrancy attack to keep receiving the collateral assets from the contract.

function seizeLoan(uint256[] calldata loanIds) public {
for (uint256 i = 0; i < loanIds.length; i++) {
uint256 loanId = loanIds[i];
// get the loan info
Loan memory loan = loans[loanId];
// validate the loan
if (loan.auctionStartTimestamp == type(uint256).max)
revert AuctionNotStarted();
if (
block.timestamp <
loan.auctionStartTimestamp + loan.auctionLength
) revert AuctionNotEnded();
// calculate the fee
uint256 govFee = (borrowerFee * loan.collateral) / 10000;
// transfer the protocol fee to governance
IERC20(loan.collateralToken).transfer(feeReceiver, govFee);
// transfer the collateral tokens from the contract to the lender
IERC20(loan.collateralToken).transfer(
loan.lender,
loan.collateral - govFee
);
​
bytes32 poolId = keccak256(
abi.encode(loan.lender, loan.loanToken, loan.collateralToken)
);
​
// update the pool outstanding loans
pools[poolId].outstandingLoans -= loan.debt;
​
emit LoanSiezed(
loan.borrower,
loan.lender,
loanId,
loan.collateral
);
// delete the loan
delete loans[loanId];
}
}

setPool() transferring the loanToken difference back to the lender if the param p.poolBalance < currentBalance of the pool before updating the new pool configs to the pool. However, since there is no re-entrancy guard on this function, there is a risk of re-entrancy attack to keep receiving the loan assets from the contract.

function setPool(Pool calldata p) public returns (bytes32 poolId) {
// validate the pool
if (
p.lender != msg.sender ||
p.minLoanSize == 0 ||
p.maxLoanRatio == 0 ||
p.auctionLength == 0 ||
p.auctionLength > MAX_AUCTION_LENGTH ||
p.interestRate > MAX_INTEREST_RATE
) revert PoolConfig();
​
// check if they already have a pool balance
poolId = getPoolId(p.lender, p.loanToken, p.collateralToken);
​
// you can't change the outstanding loans
if (p.outstandingLoans != pools[poolId].outstandingLoans)
revert PoolConfig();
​
uint256 currentBalance = pools[poolId].poolBalance;
​
if (p.poolBalance > currentBalance) {
// if new balance > current balance then transfer the difference from the lender
IERC20(p.loanToken).transferFrom(
p.lender,
address(this),
p.poolBalance - currentBalance
);
} else if (p.poolBalance < currentBalance) {
// if new balance < current balance then transfer the difference back to the lender
IERC20(p.loanToken).transfer(
p.lender,
currentBalance - p.poolBalance
);
}
​
emit PoolBalanceUpdated(poolId, p.poolBalance);
​
if (pools[poolId].lender == address(0)) {
// if the pool doesn't exist then create it
emit PoolCreated(poolId, p);
} else {
// if the pool does exist then update it
emit PoolUpdated(poolId, p);
}
​
pools[poolId] = p;
}

##POC
for addToPool():

Initial state: poolBalance = 0
Adding to pool 1000 loanToken amount should increase the poolBalance to 1000, but the lender can use re-entrancy to re-enter the addToPool with the same 1000 amount.

  1. Outer addToPool(1000): poolBalance = 1000. Control is given to attacker ...

  2. Inner addToPool(1000): poolBalance = 2000.

  3. Withdrawing the 2000 loanToken via removeFromPool(2000), The attacker makes a profit of 2000 - 1000 = 1000 loantoken.

  4. Repeating the attack until the loanToken assets of contract is drained.

seizeLoan() case would be the same, lender call seizeLoan(loanId) and then re-enter it until there are none of collateral assets left of that loan in the contract.

setPool() case would be the same, lender call setPool(p) with p.poolBalance < currentBalance of the pool and then re-enter it until there are none of the loanToken in the pool's config left of in the contract.

Impact

Fund losing for other lenders and borrowers.

Tools Used

Manual

Recommendations

Consider adding nonReentrant modifier from OZ's ReentrancyGuard.

Support

FAQs

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