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

Lender.sol#seizeLoan - ERC777 reentrancy risk due to missing nonReentrant modifier and not following CEI pattern

0xdeth

Summary

ERC777 reentrancy risk due to missing nonReentrant modifier and not following CEI pattern

Vulnerability Details

ERC777 tokens allow arbitrary callbacks via hooks that are called during token transfers. Malicious contracts can utilize these hooks and perform a reentrancy.

This vulnerability exists because seizeLoan doesn't adhere to the CEI pattern, we delete the loan here, while we transfer the tokens here. Also seizeLoan doesn't implement any reentrancy guard to guard against this sort of attacks.

Let's imagine the following scenario:

A lender's loan auction has ended and now his loan can be seized.
Someone calls seizeLoan to seize the loan.
Once we hit this line the transfer of tokens begins.
If the token is an ERC777 token it will execute a callback and inside that callback a malicious contract can call seizeLoan again effectively doubling the amount of collateral the lender will receive accordingly.

The almost exact same issue can be seen in the repay function.

Impact

Loss of funds for the protocol.

Tools Used

Manual review

Recommendations

Implement OpenZeppelin's ReentrancyGuard and add the nonReentrant modifier to seizeLoan.

Move the line delete loans[loanId] before the transferring of tokens.

// Use OpenZeppelin's nonReentrant modifier
function seizeLoan(uint256[] calldata loanIds) public nonReentrant {
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
​
// Create some temporary variables and use them before transfering the collateral tokens
uint256 loanCollateral = loan.collateral;
address loanLender = loan.lender;
address loanCollateralToken = loan.collateralToken;
address loanLoanToken = loan.loanToken;
uint256 loanDebt = loan.debt;
// delete the loan
delete loans[loanId];
​
uint256 govFee = (borrowerFee * loanCollateral) / 10000;
// transfer the protocol fee to governance
IERC20(loanCollateralToken).transfer(feeReceiver, govFee);
// transfer the collateral tokens from the contract to the lender
IERC20(loanCollateralToken).transfer(
loanLender,
loanCollateral - govFee
);
​
bytes32 poolId = keccak256(
abi.encode(loanLender, loanLoanToken, loanCollateralToken)
);
​
// update the pool outstanding loans
pools[poolId].outstandingLoans -= loanDebt;
​
emit LoanSiezed(
loan.borrower,
loan.lender,
loanId,
loan.collateral
);
}
}

Support

FAQs

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

Give us feedback!