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

In the `refinance()`, the new pool, whoever takes the loan, his balance will be subtracted twice

Summary

There is a counting error in the Lender.sol::refinance() function causing losses to the new pool who takes the loan.

Vulnerability Details

The Lender.sol::refinance() helps to the borrower to refinance his loan to a new offer (pool).

The problem is that the new pool, whoever takes the loan, his balance is substracted twice. First the balance is substracted here:

File: Lender.sol
635: // now lets deduct our tokens from the new pool
636: _updatePoolBalance(poolId, pools[poolId].poolBalance - debt);

and here is subtracted one more time:

File: Lender.sol
697: // update pool balance
698: pools[poolId].poolBalance -= debt;

I created a test where the new pool (initial pool balance of 1000 tokens) who takes the loan will end up with 800 token balance, which is incorrect because the loan debt is 100 tokens. Test steps:

  1. Create a borrow in the first pool with 1000 balance. Borrower borrows 100 debt and put 100 collateral.

  2. Lender2 creates a second pool with 1000 pool balance.

  3. Borrower execute the refinance to the second pool with the same debt amount.

  4. Assert first pool balance gets the 100 loaned amount, so the first pool balance is now 1000.

  5. Assert second pool balance is substracted two times and the loan debt is still 100 tokens.
    The second pool should have a balance of 900 tokens not 800.

// File: test/Lender.t.sol:LenderTest
// $ forge test --match-test "test_refinance_double_subtract_error" -vvv
//
function test_refinance_double_subtract_error() public {
// The new pool, whoever takes the loan, his balance will be subracted twice.
// 1. Create a borrow in the first pool with 1000 balance. Borrower borrows 100 debt and put 100 collateral.
// 2. Lender2 creates a second pool with 1000 pool balance.
// 3. Borrower execute the refinance to the second pool with the same debt amount.
// 4. Assert first pool balance gets the 100 loaned amount, so the first pool balance is now 1000.
// 5. Assert second pool balance is substracted two times and the loan debt is still 100 tokens.
// The second pool should have a balance of 900 tokens not 800.
//
//
// 1. Create a borrow in the first pool with 1000 balance. Borrower borrows 100 debt and put 100 collateral.
//
test_borrow();
bytes32 poolIdLender1 = lender.getPoolId(lender1, address(loanToken), address(collateralToken));
// assert loan debt, loan collateral and final pool balance
(,,,,uint256 poolBalance,,,,) = lender.pools(poolIdLender1);
(,,,,,uint256 collaterlLoan,,,,) = lender.loans(0);
assertEq(lender.getLoanDebt(0), 100 * 10**18);
assertEq(collaterlLoan, 100 * 10**18);
assertEq(poolBalance, 900 * 10**18); // 1000 pool balance - 100 loan
//
// 2. Lender2 creates a second pool with 1000 pool balance.
//
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
});
lender.setPool(p);
//
// 3. Borrower execute the refinance to the second pool with the same debt amount.
//
bytes32 poolIdLender2 = lender.getPoolId(lender2, address(loanToken), address(collateralToken));
vm.startPrank(borrower);
Refinance memory r = Refinance({
loanId: 0,
poolId: poolIdLender2,
debt: 100 * 10**18,
collateral: 100 * 10**18
});
Refinance[] memory rs = new Refinance[](1);
rs[0] = r;
lender.refinance(rs);
//
// 4. Assert first pool balance gets the 100 loaned amount, so the first pool balance is now 1000.
//
(,,,,poolBalance,,,,) = lender.pools(poolIdLender1);
assertEq(poolBalance, 1000 * 10**18);
//
// 5. Assert second pool balance is substracted two times and the loan debt is still 100 tokens.
// The second pool should have a balance of 900 tokens not 800.
//
(,,,,poolBalance,,,,) = lender.pools(poolIdLender2);
assertEq(poolBalance, 800 * 10**18); // wrong pool balance substraction
assertEq(lender.getLoanDebt(0), 100 * 10**18);
}

Impact

The new pool, whoever takes the loan, will lost balance. In the example above the borrower will pay 100 debt tokens but there are another 100 tokens that are lost and nobody will pay.

Tools used

Manual review

Recommendations

Remove the next lines:

File: Lender.sol
697: // update pool balance
698: pools[poolId].poolBalance -= debt;

Support

FAQs

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