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
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.
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));
}
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;
Pool memory pool = pools[poolId];
if (pool.lender == address(0)) revert PoolConfig();
if (debt < pool.minLoanSize) revert LoanTooSmall();
if (debt > pool.poolBalance) revert LoanTooLarge();
if (collateral == 0) revert ZeroCollateral();
uint256 loanRatio = (debt * 10 ** 18) / collateral;
if (loanRatio > pool.maxLoanRatio) revert RatioTooHigh();
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
});
_updatePoolBalance(poolId, pools[poolId].poolBalance - debt);
pools[poolId].outstandingLoans += debt;
uint256 fees = (debt * borrowerFee) / 10000;
IERC20(loan.loanToken).transfer(feeReceiver, fees);
IERC20(loan.loanToken).transfer(msg.sender, debt - fees);
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
);
}
}
src\Lender.sol
292: function repay(uint256[] calldata loanIds) public {
for (uint256 i = 0; i < loanIds.length; i++) {
uint256 loanId = loanIds[i];
Loan memory loan = loans[loanId];
(uint256 lenderInterest, uint256 protocolInterest) = _calculateInterest(loan);
bytes32 poolId = getPoolId(loan.lender, loan.loanToken, loan.collateralToken);
_updatePoolBalance(poolId, pools[poolId].poolBalance + loan.debt + lenderInterest);
pools[poolId].outstandingLoans -= loan.debt;
IERC20(loan.loanToken).transferFrom(msg.sender, address(this), loan.debt + lenderInterest);
IERC20(loan.loanToken).transferFrom(msg.sender, feeReceiver, protocolInterest);
IERC20(loan.collateralToken).transfer(loan.borrower, loan.collateral);
emit Repaid(
msg.sender, loan.lender, loanId, loan.debt, loan.collateral, loan.interestRate, loan.startTimestamp
);
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];
Loan memory loan = loans[loanId];
(uint256 lenderInterest, uint256 protocolInterest) = _calculateInterest(loan);
bytes32 poolId = getPoolId(loan.lender, loan.loanToken, loan.collateralToken);
_updatePoolBalance(poolId, pools[poolId].poolBalance + loan.debt + lenderInterest);
pools[poolId].outstandingLoans -= loan.debt;
+
+ delete loans[loanId];
IERC20(loan.loanToken).transferFrom(msg.sender, address(this), loan.debt + lenderInterest);
IERC20(loan.loanToken).transferFrom(msg.sender, feeReceiver, protocolInterest);
IERC20(loan.collateralToken).transfer(loan.borrower, loan.collateral);
emit Repaid(
msg.sender, loan.lender, loanId, loan.debt, loan.collateral, loan.interestRate, loan.startTimestamp
);
-
- 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);
}