Description
borrow() allows borrower to have a debt greater than their collateral allows due to incorrect calculation. It should be something like (See Mitigation section for a better fix):
File: contracts/core/pools/LendingPool/LendingPool.sol
325: function borrow(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) {
326: if (isUnderLiquidation[msg.sender]) revert CannotBorrowUnderLiquidation();
327:
328: UserData storage user = userData[msg.sender];
329:
330: uint256 collateralValue = getUserCollateralValue(msg.sender);
331:
332: if (collateralValue == 0) revert NoCollateral();
333:
334: // Update reserve state before borrowing
335: ReserveLibrary.updateReserveState(reserve, rateData);
336:
337: // Ensure sufficient liquidity is available
338: _ensureLiquidity(amount);
339:
340: // Fetch user's total debt after borrowing
341: uint256 userTotalDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex) + amount;
342:
343: // Ensure the user has enough collateral to cover the new debt
- 344: if (collateralValue < userTotalDebt.percentMul(liquidationThreshold)) {
+ 344: if (collateralValue.percentMul(liquidationThreshold) < userTotalDebt) {
345: revert NotEnoughCollateralToBorrow();
346: }
347:
Attack Path:
Impact
User can leave the protocol underwater and extract profit.
Proof of Concept
Modify this existing test and run to see it pass, even though it should have failed with error NotEnoughCollateralToBorrow:
it("should allow user to borrow crvUSD using NFT collateral", async function () {
- const borrowAmount = ethers.parseEther("50");
+ const borrowAmount = ethers.parseEther("120");
await lendingPool.connect(user1).borrow(borrowAmount);
const crvUSDBalance = await crvusd.balanceOf(user1.address);
- expect(crvUSDBalance).to.equal(ethers.parseEther("1050"));
+ expect(crvUSDBalance).to.equal(ethers.parseEther("1120"));
const debtBalance = await debtToken.balanceOf(user1.address);
expect(debtBalance).to.gte(borrowAmount);
+
+ console.log("liquidationThreshold in bips =", await lendingPool.liquidationThreshold(), "\n");
+ console.log("debtBalance =", debtBalance);
+ console.log("collateralValue =", await lendingPool.getUserCollateralValue(user1.address));
});
Output:
LendingPool
Borrow and Repay
liquidationThreshold in bips = 8000n
debtBalance = 120000000000000000000n
collateralValue = 100000000000000000000n
✔ should allow user to borrow crvUSD using NFT collateral
1 passing (8s)
Mitigation
While the fix shown in the Description section works, the protocol should really take into account the health factor while checking borrow eligibility and hence make the following change:
File: contracts/core/pools/LendingPool/LendingPool.sol
325: function borrow(uint256 amount) external nonReentrant whenNotPaused onlyValidAmount(amount) {
326: if (isUnderLiquidation[msg.sender]) revert CannotBorrowUnderLiquidation();
327:
328: UserData storage user = userData[msg.sender];
329:
330: uint256 collateralValue = getUserCollateralValue(msg.sender);
331:
332: if (collateralValue == 0) revert NoCollateral();
333:
334: // Update reserve state before borrowing
335: ReserveLibrary.updateReserveState(reserve, rateData);
336:
337: // Ensure sufficient liquidity is available
338: _ensureLiquidity(amount);
339:
340: // Fetch user's total debt after borrowing
341: uint256 userTotalDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex) + amount;
342:
343: // Ensure the user has enough collateral to cover the new debt
- 344: if (collateralValue < userTotalDebt.percentMul(liquidationThreshold)) {
+ 344: if (calculateHealthFactor(msg.sender) < healthFactorLiquidationThreshold) {
345: revert NotEnoughCollateralToBorrow();
346: }
347:
