Core Contracts
Regnum Aurum Acquisition Corp
HardhatReal World AssetsNFT
77,280 USDC
View results
Previous Next
Submission Details
Severity: high
Valid

Over-Liquidation in LiquidationPool::finalizeLiquidation() Process leads to loss of funds for borrower

mill1995

Summary

The finalizeLiquidation function in the LendingPool contract liquidates all NFT collateral from a borrower regardless of the debt amount or minimum collateral needed to restore the health factor. This results in unnecessary loss of collateral for borrowers.

Vulnerability Details

The vulnerability exists in the finalizeLiquidation function:

function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {
// Transfer NFTs to Stability Pool
for (uint256 i = 0; i < user.nftTokenIds.length; i++) {
uint256 tokenId = user.nftTokenIds[i];
user.depositedNFTs[tokenId] = false;
raacNFT.transferFrom(address(this), stabilityPool, tokenId);
}
delete user.nftTokenIds;
}

The issue can be demonstrated with the following scenario from the Proof of Concept:

  • User deposits 5 NFTs worth 100e18 each (total 500e18)

  • User borrows 400e18 (80% of collateral) = health factor of 1

  • One NFT's price drops by 5%, making total collateral 495e18

  • Position becomes liquidatable (health factor < 1)

  • During liquidation, ALL NFTs are transferred to the Stability Pool, even though liquidating just the devalued NFT would be sufficient to restore a positive health factor

PoC

This test assumes that the issue in the StabilityPool::liquidateBorrower() function for the approval has been fixed (see issue: "StabilityPool can't liquidate positions because of wrong user debt amount being approved causing the transaction to fail")

For the purpose of this test I modified the function to approve type(uint256).max. This shouldn't be done in production and there is already a recommendation in the issue mentioned above.

Update Line 461 in the liquidateBorrower() function :

function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
- bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);
+ bool approveSuccess = crvUSDToken.approve(address(lendingPool), type(uint256).max);
}
`

In order to run the test you need to:

  1. Run foundryup to get the latest version of Foundry

  2. Install hardhat-foundry: npm install --save-dev @nomicfoundation/hardhat-foundry

  3. Import it in your Hardhat config: require("@nomicfoundation/hardhat-foundry");

  4. Make sure you've set the BASE_RPC_URL in the .env file or comment out the forking option in the hardhat config.

  5. Run npx hardhat init-foundry

  6. There is one file in the test folder that will throw an error during compilation so rename the file in test/unit/libraries/ReserveLibraryMock.sol to => ReserveLibraryMock.sol_broken so it doesn't get compiled anymore (we don't need it anyways).

  7. Create a new folder test/foundry

  8. Paste the below code into a new test file i.e.: FoundryTest.t.sol

  9. Run the test: forge test --mc FoundryTest -vvvv

// SPDX-License-Identifier: UNLICENSED
pragma solidity ^0.8.19;
​
import {Test} from "forge-std/Test.sol";
import {console2} from "forge-std/console2.sol";
​
import {StabilityPool} from "../../contracts/core/pools/StabilityPool/StabilityPool.sol";
import {crvUSDToken} from "../../contracts/mocks/core/tokens/crvUSDToken.sol";
import {RAACToken} from "../../contracts/core/tokens/RAACToken.sol";
import {RAACHousePrices} from "../../contracts/core/primitives/RAACHousePrices.sol";
import {RAACNFT} from "../../contracts/core/tokens/RAACNFT.sol";
import {RToken} from "../../contracts/core/tokens/RToken.sol";
import {DebtToken} from "../../contracts/core/tokens/DebtToken.sol";
import {DEToken} from "../../contracts/core/tokens/DEToken.sol";
import {LendingPool} from "../../contracts/core/pools/LendingPool/LendingPool.sol";
import {RAACMinter, IRAACMinter} from "../../contracts/core/minters/RAACMinter/RAACMinter.sol";
import {PercentageMath} from "../../contracts/libraries/math/PercentageMath.sol";
import {ILendingPool} from "../../contracts/interfaces/core/pools/LendingPool/ILendingPool.sol";
import {IStabilityPool} from "../../contracts/interfaces/core/pools/StabilityPool/IStabilityPool.sol";
​
contract FoundryTest is Test {
using PercentageMath for uint256;
​
StabilityPool public stabilityPool;
LendingPool public lendingPool;
RAACMinter public raacMinter;
​
crvUSDToken public crvusd;
RToken public rToken;
DEToken public deToken;
RAACToken public raacToken;
RAACNFT public raacNFT;
DebtToken public debtToken;
RAACHousePrices public raacHousePrices;
​
address public owner;
address public user1;
address public user2;
address public user3;
address public treasury;
​
uint256 public constant INITIAL_BALANCE = 1000e18;
uint256 public constant INITIAL_PRIME_RATE = 1e27;
​
uint256 constant INITIAL_BATCH_SIZE = 3;
uint256 constant HOUSE_PRICE = 100e18;
uint256 constant TOKEN_ID = 1;
​
function setUp() public {
// Setup accounts
owner = address(this);
user1 = makeAddr("user1");
user2 = makeAddr("user2");
user3 = makeAddr("user3");
treasury = makeAddr("treasury");
​
// Deploy base tokens
crvusd = new crvUSDToken(owner);
crvusd.setMinter(owner);
​
raacToken = new RAACToken(owner, 100, 50);
​
// Deploy price oracle and set oracle
raacHousePrices = new RAACHousePrices(owner);
raacHousePrices.setOracle(owner);
​
// Set initial house prices
raacHousePrices.setHousePrice(TOKEN_ID, HOUSE_PRICE);
​
// Deploy NFT
raacNFT = new RAACNFT(address(crvusd), address(raacHousePrices), owner);
​
// Deploy pool tokens
rToken = new RToken("RToken", "RToken", owner, address(crvusd));
debtToken = new DebtToken("DebtToken", "DT", owner);
deToken = new DEToken("DEToken", "DEToken", owner, address(rToken));
​
// Deploy pools
lendingPool = new LendingPool(
address(crvusd),
address(rToken),
address(debtToken),
address(raacNFT),
address(raacHousePrices),
INITIAL_PRIME_RATE
);
​
stabilityPool = new StabilityPool(owner);
​
// this is needed otherwise lastEmissionUpdateTimestamp will underflow in the RAACMinter constructor
vm.warp(block.timestamp + 2 days);
​
// Deploy RAAC minter
raacMinter = new RAACMinter(address(raacToken), address(stabilityPool), address(lendingPool), owner);
​
// Setup cross-contract references
lendingPool.setStabilityPool(address(stabilityPool));
rToken.setReservePool(address(lendingPool));
debtToken.setReservePool(address(lendingPool));
rToken.transferOwnership(address(lendingPool));
debtToken.transferOwnership(address(lendingPool));
​
deToken.setStabilityPool(address(stabilityPool));
deToken.transferOwnership(address(stabilityPool));
​
// Initialize Stability Pool
stabilityPool.initialize(
address(rToken),
address(deToken),
address(raacToken),
address(raacMinter),
address(crvusd),
address(lendingPool)
);
​
// Setup permissions
raacToken.setMinter(address(raacMinter));
raacToken.manageWhitelist(address(stabilityPool), true);
​
// Mint initial tokens and setup approvals
_setupInitialBalancesAndAllowances();
}
​
function test_AllNFtsGetLiquidated() public {
// totalLiquidity 3000e18
// totalUsage 0
_printReserveState();
​
uint256 NFT_COUNT = 5;
// setup NFTs
for (uint256 i = 0; i < NFT_COUNT; i++) {
raacHousePrices.setHousePrice(TOKEN_ID + i, HOUSE_PRICE);
}
​
// Setup borrower
address borrower = makeAddr("borrower");
crvusd.mint(borrower, HOUSE_PRICE * NFT_COUNT);
assertEq(crvusd.balanceOf(borrower), HOUSE_PRICE * NFT_COUNT);
​
// mint NFTs to borrower
vm.startPrank(borrower);
crvusd.approve(address(raacNFT), HOUSE_PRICE * NFT_COUNT);
​
for (uint256 i = 0; i < NFT_COUNT; i++) {
raacNFT.mint(TOKEN_ID + i, HOUSE_PRICE);
raacNFT.approve(address(lendingPool), TOKEN_ID + i);
// Deposit all NFTs to the LendingPool as collateral
lendingPool.depositNFT(TOKEN_ID + i);
}
assertEq(raacNFT.balanceOf(address(lendingPool)), NFT_COUNT);
// total collateral value is 500e18 (5 NFTS * 100e18)
uint256 totalCollateralValue = lendingPool.getUserCollateralValue(borrower);
console2.log("totalCollateralValue at start", totalCollateralValue);
// borrow 80 % of our collateral which is 400e18
lendingPool.borrow(totalCollateralValue.percentMul(lendingPool.liquidationThreshold()));
vm.stopPrank();
​
// get current health factor which is 1 so we have a healthy position right now
uint256 healthFactor = lendingPool.calculateHealthFactor(borrower);
console2.log("healthFactor at start", healthFactor);
​
// now the price of one NFT drops by 5%
raacHousePrices.setHousePrice(TOKEN_ID, (HOUSE_PRICE * 95) / 100);
​
// total collateral value is 495e18
totalCollateralValue = lendingPool.getUserCollateralValue(borrower);
console2.log("totalCollateralValue after price drops by 5%", totalCollateralValue);
​
// get current health factor which is 0.99 so we have an unhealthy position now which can be liquidated
healthFactor = lendingPool.calculateHealthFactor(borrower);
console2.log("healthFactor after price drops by 5%", healthFactor);
​
// liquidation gets initiated
lendingPool.initiateLiquidation(borrower);
assertEq(lendingPool.isUnderLiquidation(borrower), true);
​
// wait for the grace period to expire
vm.warp(lendingPool.liquidationStartTime(borrower) + lendingPool.liquidationGracePeriod() + 1 seconds);
​
// mint the necessary amount of crvUSD to repay the debt + some extra to cover the interest
crvusd.mint(address(stabilityPool), 410e18);
​
// liquidation gets finalized
// All NFTs get liquidated even though it would only be necessary to liquidate the lowest value NFT
stabilityPool.liquidateBorrower(borrower);
​
_printReserveState();
}
​
function _setupInitialBalancesAndAllowances() internal {
// Mint crvUSD to users
crvusd.mint(user1, INITIAL_BALANCE);
crvusd.mint(user2, INITIAL_BALANCE);
crvusd.mint(user3, INITIAL_BALANCE);
​
// Setup approvals for users
vm.startPrank(user1);
crvusd.approve(address(lendingPool), type(uint256).max);
lendingPool.deposit(INITIAL_BALANCE);
rToken.approve(address(stabilityPool), type(uint256).max);
vm.stopPrank();
​
vm.startPrank(user2);
crvusd.approve(address(lendingPool), type(uint256).max);
lendingPool.deposit(INITIAL_BALANCE);
rToken.approve(address(stabilityPool), type(uint256).max);
vm.stopPrank();
​
vm.startPrank(user3);
crvusd.approve(address(lendingPool), type(uint256).max);
lendingPool.deposit(INITIAL_BALANCE);
rToken.approve(address(stabilityPool), type(uint256).max);
vm.stopPrank();
}
​
function _printReserveState() internal view {
(
address reserveRTokenAddr,
address reserveAssetAddr,
address reserveDebtTokenAddr,
uint256 totalLiquidity,
uint256 totalUsage,
uint128 liquidityIndex,
uint128 usageIndex,
uint40 lastUpdateTimestamp
) = lendingPool.reserve();
​
console2.log("Reserve Data");
console2.log("totalLiquidity", totalLiquidity);
console2.log("totalUsage", totalUsage);
console2.log("liquidityIndex", liquidityIndex);
console2.log("usageIndex", usageIndex);
console2.log("lastUpdateTimestamp", lastUpdateTimestamp);
console2.log("================================================");
}
​
function _printRateData() internal view {
(
uint256 currentLiquidityRate,
uint256 currentUsageRate,
uint256 primeRate,
uint256 baseRate,
uint256 optimalRate,
uint256 maxRate,
uint256 optimalUtilizationRate,
uint256 protocolFeeRate
) = lendingPool.rateData();
​
console2.log("Rate Data");
console2.log("currentLiquidityRate", currentLiquidityRate);
console2.log("currentUsageRate", currentUsageRate);
console2.log("primeRate", primeRate);
console2.log("baseRate", baseRate);
console2.log("optimalRate", optimalRate);
console2.log("maxRate", maxRate);
console2.log("optimalUtilizationRate", optimalUtilizationRate);
console2.log("protocolFeeRate", protocolFeeRate);
console2.log("================================================");
}
}

To demonstrate that it's sufficient to only transfer the lowest valued NFT to restore a positive health factor you can follow these steps (! Please note that this is only for demonstration purpose !) :

  • Modify the finalizeLiquidation function to only liquidate the lowest valued NFT:

function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
​
// update state
ReserveLibrary.updateReserveState(reserve, rateData);
​
if (block.timestamp <= liquidationStartTime[userAddress] + liquidationGracePeriod) {
revert GracePeriodNotExpired();
}
​
UserData storage user = userData[userAddress];
​
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
​
isUnderLiquidation[userAddress] = false;
liquidationStartTime[userAddress] = 0;
- // Transfer NFTs to Stability Pool
- for (uint256 i = 0; i < user.nftTokenIds.length; i++) {
- uint256 tokenId = user.nftTokenIds[i];
- user.depositedNFTs[tokenId] = false;
- raacNFT.transferFrom(address(this), stabilityPool, tokenId);
- }
- delete user.nftTokenIds;
​
// Only transfer one NFT, we know that it's at position 0 in the array
+ raacNFT.transferFrom(address(this), stabilityPool, user.nftTokenIds[0]);
+ uint256 price = getNFTPrice(user.nftTokenIds[0]);
+ user.nftTokenIds[0] = user.nftTokenIds[user.nftTokenIds.length - 1];
+ user.nftTokenIds.pop();
​
- // Burn DebtTokens from the user
- (uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) = IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, userDebt, reserve.usageIndex);
+ // Burn the value of the collateral that gets liquidated (price of NFT)
+ (uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) = IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, price, reserve.usageIndex);
​
// Transfer reserve assets from Stability Pool to cover the debt
IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);
​
// Update user's scaled debt balance
user.scaledDebtBalance -= amountBurned;
reserve.totalUsage = newTotalSupply;
​
// Update liquidity and interest rates
ReserveLibrary.updateInterestRatesAndLiquidity(reserve, rateData, amountScaled, 0);
​
​
emit LiquidationFinalized(stabilityPool, userAddress, userDebt, getUserCollateralValue(userAddress));
}

  • Then add this test to show that the health factor will be positive:

function test_LiquidateLowestValueNFT() public {
// totalLiquidity 3000e18
// totalUsage 0
_printReserveState();
​
uint256 NFT_COUNT = 5;
// setup many NFTs
for (uint256 i = 0; i < NFT_COUNT; i++) {
raacHousePrices.setHousePrice(TOKEN_ID + i, HOUSE_PRICE);
}
​
// Setup borrower
address borrower = makeAddr("borrower");
crvusd.mint(borrower, HOUSE_PRICE * NFT_COUNT);
assertEq(crvusd.balanceOf(borrower), HOUSE_PRICE * NFT_COUNT);
​
// mint nft to borrower for 100e18
vm.startPrank(borrower);
crvusd.approve(address(raacNFT), HOUSE_PRICE * NFT_COUNT);
​
for (uint256 i = 0; i < NFT_COUNT; i++) {
raacNFT.mint(TOKEN_ID + i, HOUSE_PRICE);
raacNFT.approve(address(lendingPool), TOKEN_ID + i);
lendingPool.depositNFT(TOKEN_ID + i);
}
assertEq(raacNFT.balanceOf(address(lendingPool)), NFT_COUNT);
// total collateral value is 500e18
uint256 totalCollateralValue = lendingPool.getUserCollateralValue(borrower);
console2.log("totalCollateralValue at start", totalCollateralValue);
// borrow 80 % of our collateral which is 400e18
lendingPool.borrow(totalCollateralValue.percentMul(lendingPool.liquidationThreshold()));
vm.stopPrank();
​
uint256 totalDebt = lendingPool.getUserDebt(borrower);
console2.log("totalDebt at start", totalDebt);
​
// get current health factor which is 1 so we have a healthy position right now
uint256 healthFactor = lendingPool.calculateHealthFactor(borrower);
console2.log("healthFactor at start", healthFactor);
​
// now the price of one NFT drops by 5%
raacHousePrices.setHousePrice(TOKEN_ID, (HOUSE_PRICE * 95) / 100);
​
// total collateral value is 495e18
totalCollateralValue = lendingPool.getUserCollateralValue(borrower);
console2.log("totalCollateralValue after price drops by 5%", totalCollateralValue);
​
// get current health factor which is 0.99 so we have an unhealthy position now which can be liquidated
healthFactor = lendingPool.calculateHealthFactor(borrower);
console2.log("healthFactor after price drops by 5%", healthFactor);
​
// liquidation gets initiated
lendingPool.initiateLiquidation(borrower);
assertEq(lendingPool.isUnderLiquidation(borrower), true);
​
// wait for the grace period to expire
vm.warp(lendingPool.liquidationStartTime(borrower) + lendingPool.liquidationGracePeriod() + 1 seconds);
​
// mint the necessary amount of crvUSD to repay the debt + some extra to cover the interest
crvusd.mint(address(stabilityPool), 410e18);
​
// liquidation gets finalized
//!!! Only one NFT gets liquidated though !!!
stabilityPool.liquidateBorrower(borrower);
​
_printReserveState();
​
// health factor is 1.044 now so position is positive
healthFactor = lendingPool.calculateHealthFactor(borrower);
console2.log("healthFactor after liquidation", healthFactor);
​
// collateral value decreased to 400e18 because we remove the lowest valued NFT
totalCollateralValue = lendingPool.getUserCollateralValue(borrower);
console2.log("totalCollateralValue after liquidation", totalCollateralValue);
​
totalDebt = lendingPool.getUserDebt(borrower);
// user still has some debt because his position is still open
console2.log("totalDebt after liquidation", totalDebt);
}

Impact

  • Borrowers lose more collateral than necessary to cover their debt

  • Creates unfair economic outcomes for borrowers

Tools Used

  • Foundry

  • Manual Review

Recommendations

Calculate the minimum number of NFTs needed to restore the health factor above the threshold, rather than liquidating all NFTs. You could sort the NFTs in ascending order and then calculate how many NFTs are needed.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 2 months ago
Submission Judgement Published
Validated
Assigned finding tags:

Users can deposit NFTs using LendingPool::depositNFT while under liquidation, leading to unfair liquidation of NFTs that weren't part of original position

Support

FAQs

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