Description
While the pause functionality in the contract is necessary to prevent eventual attacks, it is still necessary though to keep it fair to the users as well as symmetrical. Within the current implementation there is a scenario in which the Lending Pool will undergo a pause, but the Stability Pool is not, leading to unfair liquidations of users.
Vulnerability Details
Both, LendingPool
and StabilityPool
have the functionality to pause each contract in the case of an unforeseen event. However, the pause states are not connected, which, in combination with the unusual dual contract liquidation logic, creates an edge case, in which users are not able to repay their debt and close liquidation of their positions, but can still be liquidated. This would of course lead to potentially unfair liquidations, if the users have the funds and intend to repay their debts.
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_pocUnfairLiquidationDuringPause -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));
attacker = new Attacker(address(raacNFT));
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_pocUnfairLiquidationDuringPause() 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);
vm.expectRevert();
lendingPool.repay(9e18);
vm.expectRevert();
lendingPool.closeLiquidation();
vm.stopPrank();
crvusd.mint(address(stabilityPool), 100e18);
vm.startPrank(owner);
lendingPool.setStabilityPool(address(stabilityPool));
stabilityPool.liquidateBorrower(user1);
vm.stopPrank();
}
}
Impact
A user, even though he might have available funds and intend, could lose his NFT due to the inconsistency in state. While it would be possible for him to buy it back on a premium, he would still incur a potentially severe financial loss, or lose his real estate. Therefore I rate the impact as high.
The likelihood however is to be considered a medium, since it requires pre-conditions.
This leads to a total severity rating of High, nevertheless.
Tools Used
Manual Review
Recommended Mitigation
Ensure the finalizeLiquidation
function will not / can not be executed within that timeframe:
- function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {
+ function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool whenNotPaused {
if (!isUnderLiquidation[userAddress]) revert NotUnderLiquidation();
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;
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