Beedle - Oracle free perpetual lending
BeedleFi
DeFiFoundry
20,000 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

New lender loses fund due to wrong calculation in refinance() function

sonny2k

Summary

When a borrower calls refinance(), the loan will be brought to the new pool of borrower's choice. While the poolBalance of the old pool is correctly updated, the poolBalance of the new pool is wrongly calculated, which lead to permanent fund losing of the new lender.

Vulnerability Details

The poolBalance of the new pool is deducted twice, which should be only once.

First it occurs here: https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L636

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

But then in the end when updating loans[loanID] states, the poolBalance is mistakenly deducted once again: https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L698

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

POC

Copy this test case into test/Lender.t.sol

function test_refinancePoc() public {
test_borrow();
​
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);
​
vm.startPrank(borrower);
Refinance memory r = Refinance({
loanId: 0,
poolId: keccak256(
abi.encode(
address(lender2),
address(loanToken),
address(collateralToken)
)
),
debt: 100*10**18,
collateral: 100*10**18
});
Refinance[] memory rs = new Refinance[](1);
rs[0] = r;
​
lender.refinance(rs);
​
bytes32[] memory poolIds = new bytes32[](2);
poolIds[0] = keccak256(
abi.encode(
address(lender2),
address(loanToken),
address(collateralToken)
)
);
​
poolIds[1] = keccak256(
abi.encode(
address(lender1),
address(loanToken),
address(collateralToken)
)
);
​
// check if borrower still has the same loan token as debt after refinancing, borrow fee is 50 so that borrower only has 995*10*17
assertEq(loanToken.balanceOf(address(borrower)), 995*10**17);
​
// right here the poolBalance of the new pool is deducted twice of the debt amount, the correct poolBalance should be 900*10**18 since the debt amount of the old pool is only 100*10**18
(,,,,uint256 poolBalance,,,,) = lender.pools(poolIds[0]);
assertEq(poolBalance, 800*10**18);
​
// the old pool is successfully refunded by the debt amount, as originally 1000*10**18 when the lender1 first set up the pool
(,,,,uint256 poolBalance1,,,,) = lender.pools(poolIds[1]);
assertEq(poolBalance1, 1000*10**18);
}
​

Use forge test --mt test_refinancePoc to run this test case.

Impact

The lender of the new pool will permanently lose the fund equals to the loan debt amount. Or the borrower's refinance action will revert without a specific reason as the poolBalance of the new pool is underflow when poolBalance < loan.debt * 2 since it will be deducted twice. Which also leads to the fact that the new lender loses his/her possibility of earning interest yield although his/her poolBalance is enough for the loan to be refinanced to.

Tools Used

Manual

Recommendations

  1. Remove the second deduction of the new pool's poolBalance.
    https://github.com/Cyfrin/2023-07-beedle/blob/main/src/Lender.sol#L698

- // update pool balance
- pools[poolId].poolBalance -= debt;

Support

FAQs

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