Summary
In the StabilityPool::liquidateBorrower function of the StabilityPool contract, user debt is incorrectly scaled twice - once when retrieved from the lending pool (already scaled) and again in the stability pool, leading to excessive assets needed for liquidation.
Vulnerability Details
The vulnerability exists in the liquidateBorrower function:
function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
_update();
@> uint256 userDebt = lendingPool.getUserDebt(userAddress);
@> uint256 scaledUserDebt = WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt());
if (userDebt == 0) revert InvalidAmount();
}
The userDebt returned from lendingPool.getUserDebt() is already scaled by the normalized debt factor. However, the code scales it again by multiplying with lendingPool.getNormalizedDebt(), resulting in the debt being inflated.
Impact
This double scaling has the following consequences:
Excessive collateral is needed for liquidation
A liquidation could not be completed even if the stability pool has enough assets to do it, causing a possible insolvency on the lending pool
Tools Used
Manual Review
Proof of Concept
Add the following test case to the test/e2e/protocols-tests.js file in the StabilityPool section:
it('should demonstrate double scaling of debt', async function () {
await contracts.stabilityPool.connect(user1).deposit(STABILITY_DEPOSIT);
await contracts.crvUSD.connect(user3).approve(contracts.stabilityPool.target, STABILITY_DEPOSIT);
await contracts.crvUSD.connect(user3).transfer(contracts.stabilityPool.target, STABILITY_DEPOSIT);
const newTokenId = HOUSE_TOKEN_ID + 2;
await contracts.housePrices.setHousePrice(newTokenId, HOUSE_PRICE);
await contracts.crvUSD.connect(user2).approve(contracts.nft.target, HOUSE_PRICE);
await contracts.nft.connect(user2).mint(newTokenId, HOUSE_PRICE);
await contracts.nft.connect(user2).approve(contracts.lendingPool.target, newTokenId);
await contracts.lendingPool.connect(user2).depositNFT(newTokenId);
await contracts.lendingPool.connect(user2).borrow(BORROW_AMOUNT);
await contracts.housePrices.setHousePrice(newTokenId, HOUSE_PRICE * 10n / 100n);
await contracts.lendingPool.connect(user3).initiateLiquidation(user2.address);
await time.increase(73 * 60 * 60);
const userDebtBalanceOf = await contracts.debtToken.balanceOf(user2.address);
const userDebt = await contracts.lendingPool.getUserDebt(user2.address);
expect(userDebt).to.be.eq(userDebtBalanceOf);
});
Recommendations
Remove the second scaling in the StabilityPool contract since the debt is already scaled when received from LendingPool:
function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
_update();
// Get the user's debt from the LendingPool.
- uint256 userDebt = lendingPool.getUserDebt(userAddress);
- uint256 scaledUserDebt = WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt());
+ uint256 scaledUserDebt = lendingPool.getUserDebt(userAddress);
- if (userDebt == 0) revert InvalidAmount();
+ if (scaledUserDebt == 0) revert InvalidAmount();
uint256 crvUSDBalance = crvUSDToken.balanceOf(address(this));
if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance();
// Approve the LendingPool to transfer the debt amount
bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);
if (!approveSuccess) revert ApprovalFailed();
// Update lending pool state before liquidation
lendingPool.updateState();
// Call finalizeLiquidation on LendingPool
lendingPool.finalizeLiquidation(userAddress);
emit BorrowerLiquidated(userAddress, scaledUserDebt);
}
