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

LendingPool.getUserDebt returns outdated value and can lead to liquidation failure

glightspeed2

Summary

In lending pool, user's debt gets accrued over time. Since LendingPool.getUserDebtis a view function, it cannot update reserve state, so fundamentally it will return obsolete value. This might result in liquidation failure from StabilityPool.

Vulnerability Details

Root Cause Analysis

Since LendingPool.getUserDebtis a view function, it cannot update reserve state, so it returns obsolete value.

This leads not only leads to bad user experience but also to protocol failure. More specifically, when Stability Pool liquidates a borrow position:

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());
​
if (userDebt == 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);
}

It consults current user debt from LendingPool first and then update the lending pool state.

However, since lending pool is not updated, userDebtand lendingPool.getNormalizedDebtwill return obsolete values.

After updating lending pool state, the actual debt will be larger than the amount StabilityPool estimated and approved.

This can lead to liquidation failure due to ERC20InsufficientAllowance in LendingPool, because LendingPool has to transfer debt amount from StabilityPool to RToken.

Indeed, lending pool state should be updated before consulting user's debt.

POC

  • First, integrate foundry into the project

  • Next, put the following content into test/poc.t.sol and run forge test poc.t.sol -vvvv

pragma solidity ^0.8.19;
​
import "../lib/forge-std/src/Test.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 {RAACToken} from "../contracts/core/tokens/RAACToken.sol";
import {RAACNFT} from "../contracts/core/tokens/RAACNFT.sol";
import {LendingPool} from "../contracts/core/pools/LendingPool/LendingPool.sol";
import {StabilityPool} from "../contracts/core/pools/StabilityPool/StabilityPool.sol";
import {IStabilityPool} from "../contracts/interfaces/core/pools/StabilityPool/IStabilityPool.sol";
import {RAACMinter} from "../contracts/core/minters/RAACMinter/RAACMinter.sol";
import {crvUSDToken} from "../contracts/mocks/core/tokens/crvUSDToken.sol";
import "../contracts/libraries/math/WadRayMath.sol";
import "@openzeppelin/contracts/token/ERC20/IERC20.sol";
​
contract RAACHousePricesMock {
mapping(uint256 => uint256) public prices;
​
function getLatestPrice(uint256 tokenId) external view returns (uint256, uint256) {
return (prices[tokenId], block.timestamp);
}
​
function setTokenPrice(uint256 tokenId, uint256 price) external {
prices[tokenId] = price;
}
​
function tokenToHousePrice(uint256 tokenId) external view returns (uint256) {
return prices[tokenId];
}
}
​
contract LendingPoolTest is Test {
using WadRayMath for uint256;
​
RToken rtoken;
DebtToken debtToken;
RAACToken raacToken;
DEToken deToken;
RAACNFT raacNft;
RAACMinter raacMinter;
crvUSDToken asset;
​
LendingPool lendingPool;
StabilityPool stabilityPool;
RAACHousePricesMock housePrice;
​
address depositor = makeAddr("depositor");
address borrower = makeAddr("borrower");
address user = makeAddr("user");
uint256 userAssetAmount = 10_000e18;
uint256 nftPrice = 50_000e18;
​
uint256 initialBurnTaxRate = 50;
uint256 initialSwapTaxRate = 100;
uint256 initialPrimeRate = 0.1e27;
​
function setUp() external {
vm.warp(1e9); // warp time stamp to avoid underflow in RAACMinter constructor
asset = new crvUSDToken(address(this));
housePrice = new RAACHousePricesMock();
debtToken = new DebtToken("DebtToken", "DTK", address(this));
rtoken = new RToken("RToken", "RTK", address(this), address(asset));
raacNft = new RAACNFT(address(asset), address(housePrice), address(this));
lendingPool = new LendingPool(
address(asset), address(rtoken), address(debtToken), address(raacNft), address(housePrice), 0.1e27
);
rtoken.setReservePool(address(lendingPool));
debtToken.setReservePool(address(lendingPool));
deToken = new DEToken("DEToken", "DET", address(this), address(rtoken));
raacToken = new RAACToken(address(this), initialSwapTaxRate, initialBurnTaxRate);
raacToken.setMinter(address(this));
stabilityPool = new StabilityPool(address(this));
stabilityPool.initialize(
address(rtoken), address(deToken), address(raacToken), address(this), address(asset), address(lendingPool)
);
lendingPool.setStabilityPool(address(stabilityPool));
raacMinter = new RAACMinter(address(raacToken), address(stabilityPool), address(lendingPool), address(this));
stabilityPool.setRAACMinter(address(raacMinter));
deToken.setStabilityPool(address(stabilityPool));
​
uint256 depositorAmount = userAssetAmount * 100;
deal(address(asset), depositor, depositorAmount);
vm.startPrank(depositor);
asset.approve(address(lendingPool), depositorAmount);
lendingPool.deposit(depositorAmount);
vm.stopPrank();
}
​
function testFailLiquidation() external {
_mintNFT(user, 1, 100_000e18);
_depositNFT(user, 1);
_borrow(user, 80_000e18);
​
skip(1 days);
​
lendingPool.initiateLiquidation(user);
// grace period passes
skip(3 days + 1);
// set StabilityPool's crvUSD balance to user's total debt
deal(address(asset), address(stabilityPool), lendingPool.getUserDebt(user).rayMul(lendingPool.getNormalizedDebt()));
// liquidation fails due to vulnerability
stabilityPool.liquidateBorrower(user);
}
​
function _mintNFT(address account, uint256 tokenId, uint256 price) internal {
housePrice.setTokenPrice(tokenId, price);
deal(address(asset), account, price);
vm.startPrank(account);
asset.approve(address(raacNft), price);
raacNft.mint(tokenId, price);
vm.stopPrank();
​
assertEq(raacNft.ownerOf(tokenId), account);
}
​
function _depositNFT(address account, uint256 tokenId) internal {
vm.startPrank(account);
raacNft.approve(address(lendingPool), tokenId);
lendingPool.depositNFT(tokenId);
vm.stopPrank();
​
assertEq(raacNft.ownerOf(tokenId), address(lendingPool));
}
​
function _borrow(address account, uint256 amount) internal {
uint256 beforeBalance = asset.balanceOf(account);
vm.startPrank(account);
lendingPool.borrow(amount);
vm.stopPrank();
uint256 afterBalance = asset.balanceOf(account);
​
assertEq(afterBalance - beforeBalance, amount);
}
}
​

Console Output

├─ [969] crvUSDToken::transferFrom(StabilityPool: [0x03A6a84cD762D9707A21605b548aaaB891562aAb], RToken: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9], 80028498307884945693900 [8.002e22])
│ │ │ └─ ← [Revert] ERC20InsufficientAllowance(0xa0Cb889707d426A7A386870A03bc70d1b0697598, 80014247843948451192455 [8.001e22], 80028498307884945693900 [8.002e22])
│ │ └─ ← [Revert] ERC20InsufficientAllowance(0xa0Cb889707d426A7A386870A03bc70d1b0697598, 80014247843948451192455 [8.001e22], 80028498307884945693900 [8.002e22])
│ └─ ← [Revert] ERC20InsufficientAllowance(0xa0Cb889707d426A7A386870A03bc70d1b0697598, 80014247843948451192455 [8.001e22], 80028498307884945693900 [8.002e22])
└─ ← [Revert] ERC20InsufficientAllowance(0xa0Cb889707d426A7A386870A03bc70d1b0697598, 80014247843948451192455 [8.001e22], 80028498307884945693900 [8.002e22])

Impact

  • End users will always get obsolete values, unless they update reserve state manually.

  • Liquidation will fail due to incorrect estimation of user's debt (ERC20InsufficientAllowance)

Tools Used

Manual Review

Recommendations

Update lending pool state before consulting user's debt:

diff --git a/contracts/core/pools/StabilityPool/StabilityPool.sol b/contracts/core/pools/StabilityPool/StabilityPool.sol
index f8e6d18..9cd9e05 100644
--- a/contracts/core/pools/StabilityPool/StabilityPool.sol
+++ b/contracts/core/pools/StabilityPool/StabilityPool.sol
@@ -448,6 +448,8 @@ contract StabilityPool is IStabilityPool, Initializable, ReentrancyGuard, Ownabl
*/
function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
_update();
+ // Update lending pool state before liquidation
+ lendingPool.updateState();
// Get the user's debt from the LendingPool.
uint256 userDebt = lendingPool.getUserDebt(userAddress);
uint256 scaledUserDebt = WadRayMath.rayMul(userDebt, lendingPool.getNormalizedDebt());
@@ -460,8 +462,6 @@ contract StabilityPool is IStabilityPool, Initializable, ReentrancyGuard, Ownabl
// 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);
​
Updates

Lead Judging Commences

inallhonesty Lead Judge 7 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

Support

FAQs

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

Give us feedback!