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

CEI is not followed which makes functions vulnerable to reentrancy

Summary

Most of the functions in Lender.sol and Staking.sol don’t follow the check-effect-interactions pattern and don’t implement any type of reentrancy guard.

Vulnerability Details

In the following functions:

  • Lender.sol

    • setPool()

    • borrow()

    • repay()

    • giveLoan()

    • buyLoan()

    • seizeLoan()

    • refinance()

  • Staking.sol

    • deposit()

    • claim()

All of these functions are open for reentrancy, they interact with external and most likely malicious ERC20s before changing the internal state of the contract.

Poc (Proof-of-Concept)

Will give some examples of the most vulnerable functions which are easy to understand.

// Here the user can call `claim` function to claim his reward for the swap.
// updateFor() will be called which is not in our interest because it's not updating anything that will prevent reentrancy
// After that we transfer WETH to msg.sender amount, and if use any malicious ERC20s can produce reentrancy
// The claimable mapping is reset after that, as well as the overall balance of WETH in the contract
// These state changes need to be made before all external calls to prevent any logic which will run these external calls
src\Staking.sol
53: function claim() external {
updateFor(msg.sender);
WETH.transfer(msg.sender, claimable[msg.sender]);
claimable[msg.sender] = 0;
balance = WETH.balanceOf(address(this));
}
// If anyone creates a pool with malicious ERC20s and then you want to borrow from this pool.
// You will call borrow() the function will go until the transfers come, if loanToken or collateralTokens are not Safe can recall this function again,
// which will lead to the feeReceiver getting the fees again, msg.sender may get more than the asked debt or this contract can drain user collateral, there are a lot of scenarios.
src\Lender.sol
232: function borrow(Borrow[] calldata borrows) public {
for (uint256 i = 0; i < borrows.length; i++) {
bytes32 poolId = borrows[i].poolId;
uint256 debt = borrows[i].debt;
uint256 collateral = borrows[i].collateral;
// get the pool info
Pool memory pool = pools[poolId];
// make sure the pool exists
if (pool.lender == address(0)) revert PoolConfig();
// validate the loan
if (debt < pool.minLoanSize) revert LoanTooSmall();
if (debt > pool.poolBalance) revert LoanTooLarge();
if (collateral == 0) revert ZeroCollateral();
// make sure the user isn't borrowing too much
uint256 loanRatio = (debt * 10 ** 18) / collateral;
if (loanRatio > pool.maxLoanRatio) revert RatioTooHigh();
// create the loan
Loan memory loan = Loan({
lender: pool.lender,
borrower: msg.sender,
loanToken: pool.loanToken,
collateralToken: pool.collateralToken,
debt: debt,
collateral: collateral,
interestRate: pool.interestRate,
startTimestamp: block.timestamp,
auctionStartTimestamp: type(uint256).max,
auctionLength: pool.auctionLength
});
// update the pool balance
_updatePoolBalance(poolId, pools[poolId].poolBalance - debt);
pools[poolId].outstandingLoans += debt;
// calculate the fees
uint256 fees = (debt * borrowerFee) / 10000;
// transfer fees
IERC20(loan.loanToken).transfer(feeReceiver, fees);
// transfer the loan tokens from the pool to the borrower
IERC20(loan.loanToken).transfer(msg.sender, debt - fees);
// transfer the collateral tokens from the borrower to the contract
IERC20(loan.collateralToken).transferFrom(msg.sender, address(this), collateral);
loans.push(loan);
emit Borrowed(
msg.sender, pool.lender, loans.length - 1, debt, collateral, pool.interestRate, block.timestamp
);
}
}
// Here you want to repay your loan, so call this function.
// It will get the loan, calculate the interest, updatePoolBalance and subtract your debt.
// then the loanToken will be transferred from you to the contract as well as interest for the protocol
//Let's assume that the collateralToken is malicious and on transferFrom will get from this contract to you and recall this
// and because the loan deletion is made after the external calls everything can be executed again until drained the whole pool.
src\Lender.sol
292: function repay(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];
// calculate the interest
(uint256 lenderInterest, uint256 protocolInterest) = _calculateInterest(loan);
bytes32 poolId = getPoolId(loan.lender, loan.loanToken, loan.collateralToken);
// update the pool balance
_updatePoolBalance(poolId, pools[poolId].poolBalance + loan.debt + lenderInterest);
pools[poolId].outstandingLoans -= loan.debt;
// transfer the loan tokens from the borrower to the pool
IERC20(loan.loanToken).transferFrom(msg.sender, address(this), loan.debt + lenderInterest);
// transfer the protocol fee to the fee receiver
IERC20(loan.loanToken).transferFrom(msg.sender, feeReceiver, protocolInterest);
// transfer the collateral tokens from the contract to the borrower
IERC20(loan.collateralToken).transfer(loan.borrower, loan.collateral);
emit Repaid(
msg.sender, loan.lender, loanId, loan.debt, loan.collateral, loan.interestRate, loan.startTimestamp
);
// delete the loan
delete loans[loanId];
}
}

Impact

Draining both lenders’ and borrowers’ funds, as well as Staking.sol contract, will make the entire protocol unusable.

Tools Used

Manual

Recommendations

Most important is to order your functions by following the CEI pattern and consider using ReentrancyGuard from OpenZeppelin. All state changes need to be made before interacting with external contracts.

Example of followed CEI on affected functions.

src\Lender.sol
292: function repay(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];
// calculate the interest
(uint256 lenderInterest, uint256 protocolInterest) = _calculateInterest(loan);
bytes32 poolId = getPoolId(loan.lender, loan.loanToken, loan.collateralToken);
// update the pool balance
_updatePoolBalance(poolId, pools[poolId].poolBalance + loan.debt + lenderInterest);
pools[poolId].outstandingLoans -= loan.debt;
+ // delete the loan
+ delete loans[loanId];
// transfer the loan tokens from the borrower to the pool
IERC20(loan.loanToken).transferFrom(msg.sender, address(this), loan.debt + lenderInterest);
// transfer the protocol fee to the fee receiver
IERC20(loan.loanToken).transferFrom(msg.sender, feeReceiver, protocolInterest);
// transfer the collateral tokens from the contract to the borrower
IERC20(loan.collateralToken).transfer(loan.borrower, loan.collateral);
emit Repaid(
msg.sender, loan.lender, loanId, loan.debt, loan.collateral, loan.interestRate, loan.startTimestamp
);
- // delete the loan
- delete loans[loanId];
}
}
src\Staking.sol
53: function claim() external {
updateFor(msg.sender);
+ uint256 reward = claimable[msg.sender];
- WETH.transfer(msg.sender, claimable[msg.sender]);
claimable[msg.sender] = 0;
balance = WETH.balanceOf(address(this));
+ WETH.transfer(msg.sender, claimable[msg.sender]);
}
src\Staking.sol
38: function deposit(uint _amount) external {
- TKN.transferFrom(msg.sender, address(this), _amount);
updateFor(msg.sender);
balances[msg.sender] += _amount;
+ TKN.transferFrom(msg.sender, address(this), _amount);
}

Support

FAQs

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