Description
While the pause() functionality is important to protect the protocol in case of unforeseen events, it should be consistent with protocol objectives. However an edge case within the consistency leads to unfair liquidations of users, if a pause() event was to be triggered after users had a liquidation initialized on their position.
Vulnerability Details
Consider the following scenario:
Bob mints an NFT worth 10e18, deposits it into the contract and takes a loan against it, for example 8e18.
Now due to accruing interest or a change in the NFT value Bob becomes due to liquidation. Said liquidation will be initialized.
Now during the Grace Period the protocol undergoes a pause() which pushes Bob over the Grace Period.
The result would now be, that Bob is unable to close the liquidation against him, even though he has the funds and intention to do so, resulting in Bobs position to be liquidated unfairly and losing him his NFT (Real Estate).
PoC
Since the PoC is a foundry test I have added a Makefile at the end of this report to simplify installation for your convenience. Otherwise if console commands would be prefered:
First run: npm install --save-dev @nomicfoundation/hardhat-foundry
Second add: require("@nomicfoundation/hardhat-foundry"); on top of the Hardhat.Config file in the projects root directory.
Third run: npx hardhat init-foundry
And lastly, you will encounter one of the mock contracts throwing an error during compilation, this error can be circumvented by commenting out the code in entirety (ReserveLibraryMocks.sol).
And the test should be good to go:
To run the following PoC please apply following "fixes" into StabilityPool as follows:
StabilityPool::liquidateBorrower
/// *** SNIP *** ///
+bool approveSuccess = crvUSDToken.approve(address(lendingPool), type(uint256).max);
-bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt;
/// *** SNIP *** ///
As mentioned in a previous reports, these issues cause liquidation under certain circumstances to be impossible.
After following above steps copy & paste the following code into ./test/invariant/PoC.t.sol and run forge test --mt test_pocUnfairLiquidationAfterPause -vv
pragma solidity ^0.8.0;
import {Test, console} from "forge-std/Test.sol";
import {StabilityPool} from "../../contracts/core/pools/StabilityPool/StabilityPool.sol";
import {LendingPool} from "../../contracts/core/pools/LendingPool/LendingPool.sol";
import {CrvUSDToken} from "../../contracts/mocks/core/tokens/crvUSDToken.sol";
import {RAACHousePrices} from "../../contracts/core/oracles/RAACHousePriceOracle.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 {RAACToken} from "../../contracts/core/tokens/RAACToken.sol";
import {RAACMinter} from "../../contracts/core/minters/RAACMinter/RAACMinter.sol";
contract PoC is Test {
StabilityPool public stabilityPool;
LendingPool public lendingPool;
CrvUSDToken public crvusd;
RAACHousePrices public raacHousePrices;
RAACNFT public raacNFT;
RToken public rToken;
DebtToken public debtToken;
DEToken public deToken;
RAACToken public raacToken;
RAACMinter public raacMinter;
address owner;
address oracle;
address user1;
address user2;
address user3;
uint256 constant STARTING_TIME = 1641070800;
uint256 public currentBlockTimestamp;
uint256 constant WAD = 1e18;
uint256 constant RAY = 1e27;
function setUp() public {
vm.warp(STARTING_TIME);
currentBlockTimestamp = block.timestamp;
owner = address(this);
oracle = makeAddr("oracle");
user1 = makeAddr("user1");
user2 = makeAddr("user2");
user3 = makeAddr("user3");
uint256 initialPrimeRate = 0.1e27;
raacHousePrices = new RAACHousePrices(owner);
vm.prank(owner);
raacHousePrices.setOracle(oracle);
crvusd = new CrvUSDToken(owner);
raacNFT = new RAACNFT(address(crvusd), address(raacHousePrices), owner);
rToken = new RToken("RToken", "RToken", owner, address(crvusd));
debtToken = new DebtToken("DebtToken", "DT", owner);
deToken = new DEToken("DEToken", "DEToken", owner, address(rToken));
vm.prank(owner);
crvusd.setMinter(owner);
vm.prank(owner);
lendingPool = new LendingPool(
address(crvusd),
address(rToken),
address(debtToken),
address(raacNFT),
address(raacHousePrices),
initialPrimeRate
);
rToken.setReservePool(address(lendingPool));
debtToken.setReservePool(address(lendingPool));
rToken.transferOwnership(address(lendingPool));
debtToken.transferOwnership(address(lendingPool));
stabilityPool = new StabilityPool(address(owner));
deToken.setStabilityPool(address(stabilityPool));
raacToken = new RAACToken(owner, 0, 0);
raacMinter = new RAACMinter(address(raacToken), address(stabilityPool), address(lendingPool), owner);
stabilityPool.initialize(address(rToken), address(deToken), address(raacToken), address(raacMinter), address(crvusd), address(lendingPool));
vm.prank(owner);
raacToken.setMinter(address(raacMinter));
crvusd.mint(address(attacker), type(uint128).max);
crvusd.mint(user1, type(uint128).max);
crvusd.mint(user2, type(uint128).max);
crvusd.mint(user3, type(uint128).max);
}
function test_pocUnfairLiquidationAfterPause() public {
vm.startPrank(user2);
crvusd.approve(address(lendingPool), type(uint256).max);
lendingPool.deposit(type(uint128).max);
vm.stopPrank();
vm.startPrank(oracle);
raacHousePrices.setHousePrice(1, 10e18);
vm.stopPrank();
vm.startPrank(user1);
crvusd.approve(address(raacNFT), type(uint256).max);
raacNFT.mint(1, 10e18);
raacNFT.approve(address(lendingPool), 1);
lendingPool.depositNFT(1);
lendingPool.borrow(8e18);
vm.stopPrank();
vm.prank(oracle);
raacHousePrices.setHousePrice(1, 5e18);
vm.startPrank(user2);
lendingPool.initiateLiquidation(user1);
vm.stopPrank();
vm.startPrank(owner);
lendingPool.pause();
vm.warp(block.timestamp + 1 + 3 days);
vm.stopPrank();
vm.startPrank(user1);
console.log(crvusd.balanceOf(user1));
crvusd.approve(address(lendingPool), 9e18);
uint256 repayAmount = debtToken.balanceOf(user1) - 1;
vm.expectRevert();
lendingPool.repay(repayAmount);
vm.expectRevert();
lendingPool.closeLiquidation();
vm.stopPrank();
crvusd.mint(address(stabilityPool), 100e18);
vm.startPrank(owner);
lendingPool.setStabilityPool(address(stabilityPool));
stabilityPool.liquidateBorrower(user1);
}
}
As you can see running above test scenario, the pause causes the user to be liquidated even though he had the funds and intention to keep his position.
Impact
Unfair liquidations are always an extensive loss to the user, while he would be able to rebuy his NFT on a premium, it would still be considered a financial loss to the user and therefore a High impact. While above scenario relies on a pause and unpause mechanic, it is certainly not a high likelihood, but certainly a medium.
Therefore I conclude a total severity of High
Tools Used
Manual Review
Recommended Mitigation
Pause events should be considered for liquidation, while it is most likely not possible to extend each users grace period, since it requires expensive looping, an if-check within the closeLiquidation and _repay function (repay is mentioned here, because of a previous report highlighting the inconsistency between repay and closeLiquidation) seems indicated:
+uint256 public lastUnpause;
function unpause() external onlyOwner {
+ lastUnpause = block.timestamp;
_unpause();
}
function closeLiquidation() external nonReentrant whenNotPaused {
address userAddress = msg.sender;
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
ReserveLibrary.updateReserveState(reserve, rateData);
+ if ( block.timestamp < lastUnpause + liquidationGracePeriod ){
+ UserData storage user = userData[userAddress];
+ uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
+ if (userDebt > DUST_THRESHOLD) revert DebtNotZero();
+ isUnderLiquidation[userAddress] = false;
+ liquidationStartTime[userAddress] = 0;
+ emit LiquidationClosed(userAddress);
+ } else if (block.timestamp > liquidationStartTime[userAddress] + liquidationGracePeriod) {
revert GracePeriodExpired();
+ } else {
UserData storage user = userData[userAddress];
uint256 userDebt = user.scaledDebtBalance.rayMul(reserve.usageIndex);
if (userDebt > DUST_THRESHOLD) revert DebtNotZero();
isUnderLiquidation[userAddress] = false;
liquidationStartTime[userAddress] = 0;
emit LiquidationClosed(userAddress);
}
}
and maybe as an additional safeguard against unfair liquidation after a pause event:
function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
ReserveLibrary.updateReserveState(reserve, rateData);
+ if (block.timestamp < lastUnpause + liquidationGracePeriod) revert GracePeriodNotExpired();
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;
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;
(uint256 amountScaled, uint256 newTotalSupply, uint256 amountBurned, uint256 balanceIncrease) = IDebtToken(reserve.reserveDebtTokenAddress).burn(userAddress, userDebt, reserve.usageIndex);
IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled); // amountBurned
user.scaledDebtBalance -= amountBurned;
reserve.totalUsage = newTotalSupply;
ReserveLibrary.updateInterestRatesAndLiquidity(reserve, rateData, amountScaled, 0);
emit LiquidationFinalized(stabilityPool, userAddress, userDebt, getUserCollateralValue(userAddress));
}
Appendix
Copy the following import into your Hardhat.Config file in the projects root dir:
require("@nomicfoundation/hardhat-foundry");
Paste the following into a new file "Makefile" into the projects root directory:
.PHONY: install-foundry init-foundry all
install-foundry:
npm install --save-dev @nomicfoundation/hardhat-foundry
init-foundry: install-foundry
npx hardhat init-foundry
# Default target that runs everything in sequence
all: install-foundry init-foundry
And run make all
