Core Contracts

Regnum Aurum Acquisition Corp

HardhatReal World AssetsNFT

77,280 USDC

View results

Previous Next

Submission Details

Severity: high

Valid

StabilityPool can't liquidate positions because of wrong user debt amount being approved causing the transaction to fail

mill1995

Summary

The StabilityPool's liquidateBorrower() function fails to properly liquidate borrowers due to using stale state data when calculating the user's debt. The function retrieves the user's debt before updating the LendingPool's state, leading to an incorrect debt calculation and failed liquidation transaction.

Currently, the state update lendingPool.updateState() is called after retrieving the user's debt and approving the token transfer, which means the debt calculation is based on outdated state information.

Vulnerability Details

Pre-Conditions:

Some user borrows liquidity from the LendingPool using his NFT as collateral (assuming the LendingPool has enough crvUSD for borrowing)
The users collateral drops in price
Someone initiates the Liquidation process
Borrower fails to repay during the grace period
Borrower can be liquidated now by the StabilityPools liquidateBorrower function

The issue occurs in the way the liquidateBorrower function calculates the userDebt amount which is used for the approve :

function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {

_update();

// !! This data is stale because the reserve state of the lending pool hasn't been updated

@> uint256 userDebt = lendingPool.getUserDebt(userAddress);

// This is also wrong because lendingPool.getUserDebt already returns the scaled amount

@> uint256 scaledUserDebt = WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt());

if (userDebt == 0) revert InvalidAmount();

uint256 crvUSDBalance = crvUSDToken.balanceOf(address(this));

if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance();

// Approve stale debt amount

@> bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);

@> if (!approveSuccess) revert ApprovalFailed();

lendingPool.updateState();

// The lending pool will use the updated user debt state due to the previous updateState call

// The finalizeLiquidation function itself also updates the reserve data

@> lendingPool.finalizeLiquidation(userAddress);

emit BorrowerLiquidated(userAddress, scaledUserDebt);

}

If we look at the LendingPool finalizeLiquidation function now we can see that it's using the updated reserve data which is using a higher amount than the previously approved amount by the StabilityPool causing the transaction to revert:

function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {

//...

// update reserve state again

@> ReserveLibrary.updateReserveState(reserve, rateData);

//...

(uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) = IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, userDebt, reserve.usageIndex);

// Transfer fails now because the Stability Pool approved the wrong token amount

@> IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);

//...

}

PoC

In order to run the test you need to:

Run foundryup to get the latest version of Foundry
Install hardhat-foundry: npm install --save-dev @nomicfoundation/hardhat-foundry
Import it in your Hardhat config: require("@nomicfoundation/hardhat-foundry");
Make sure you've set the BASE_RPC_URL in the .env file or comment out the forking option in the hardhat config.
Run npx hardhat init-foundry
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).
Create a new folder test/foundry
Paste the below code into a new test file i.e.: FoundryTest.t.sol
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

// also deposit crvUSD to the stability pool to get rTokens

_setupInitialBalancesAndAllowances();

}

function test_StabilityPoolCanNotLiquidateBorrower() public {

address borrower = makeAddr("borrower");

crvusd.mint(borrower, HOUSE_PRICE);

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

// mint nft to borrower for 100e18

vm.startPrank(borrower);

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

raacNFT.mint(TOKEN_ID, HOUSE_PRICE);

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

// deposit NFT to the lending pool

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

lendingPool.depositNFT(TOKEN_ID);

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

// Borrow

// please notice that the calculation inside the borrow function is wrong

// this is why we can borrow the same amount we provided as liquidity

// and the initiateLiquidation can happen immediately after borrowing

lendingPool.borrow(HOUSE_PRICE);

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

vm.stopPrank();

// initiate liquidation

lendingPool.initiateLiquidation(borrower);

assertEq(lendingPool.isUnderLiquidation(borrower), true);

// wait for grace period to end

vm.warp(lendingPool.liquidationStartTime(borrower) + lendingPool.liquidationGracePeriod() + 1 seconds);

// give the Stability Pool enough crvUSD to repay

crvusd.mint(address(stabilityPool), 200e18);

//!!! liquidate borrower fails with "ERC20InsufficientAllowance" because of wrong approval amount

vm.expectRevert();

stabilityPool.liquidateBorrower(borrower);

}

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

Core protocol functionality (liquidations) is broken => borrowers with bad debt can't be liquidated => high risk of protocol insolvency

Tools Used

Manual review

Foundry

Recommendations

The liquidateBorrower() function should update the LendingPool state before debt calculations, it should also remove the scaledUserDebt logic because lendingPool.getUserDebt() already returns the scaled amount:

function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {

_update();

+ lendingPool.updateState();

uint256 userDebt = lendingPool.getUserDebt(userAddress);

- uint256 scaledUserDebt = WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt());

if (userDebt == 0) revert InvalidAmount();

uint256 crvUSDBalance = crvUSDToken.balanceOf(address(this));

- if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance();

+ if (crvUSDBalance < userDebt) revert InsufficientBalance();

- bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);

+ bool approveSuccess = crvUSDToken.approve(address(lendingPool), userDebt);

if (!approveSuccess) revert ApprovalFailed();

- lendingPool.updateState();

lendingPool.finalizeLiquidation(userAddress);

- emit BorrowerLiquidated(userAddress, scaledUserDebt);

+ emit BorrowerLiquidated(userAddress, userDebt);

}

The finalizeLiquidation() function should be updated because the updateReserveState() is done in the StabilityPool:

function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {

if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();

- ReserveLibrary.updateReserveState(reserve, rateData);

}

Additional:

Implement a function in the StabilityPool to manually approve crvUSD (onlyOwner should be allowed to call) for the LendingPool as a safety mechanism

Updates

Lead Judging Commences

inallhonesty Lead Judge 4 months ago

Submission Judgement Published

Validated

Assigned finding tags:

StabilityPool: liquidateBorrower should call lendingPool.updateState earlier, to ensure the updated usageIndex is used in calculating the scaledUserDebt

StabilityPool::liquidateBorrower double-scales debt by multiplying already-scaled userDebt with usage index again, causing liquidations to fail

Prize pool breakdown

Total prize

77,280 USDC

nSLOC

3,864

20 USDC / LOC

High Medium

74,000

Low

3,280

Live

The contest is live. Earn rewards by submitting a finding.

Judging

Submissions are being carefully reviewed by our judges.

View all submissions

Appeals

This is your time to appeal against judgements on your submissions.

Appeals Review

Appeals are being carefully reviewed by our judges.

Rewards Distribution

The contest is complete and the rewards are being distributed.

View results

Support

FAQs

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