Description
For user liquidation expected behavior is that an Owner or Manager of StabilityPool calls into StabilityPool::liquidateBorrower to finalize the liquidation of an under-collateralized user after the grace period has passed. A mismatch between the approval of the crvUSD given within StabilityPool::liquidateBorrower versus the actual transfer of those tokens from LendingPool::finalizeLiquidation will cause the liquidation though to revert.
Vulnerable Code
LendingPool::finalizeLiquidation:
function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {
@> IERC20(reserve.reserveAssetAddress).safeTransferFrom(msg.sender, reserve.reserveRTokenAddress, amountScaled);
user.scaledDebtBalance -= amountBurned;
reserve.totalUsage = newTotalSupply;
ReserveLibrary.updateInterestRatesAndLiquidity(reserve, rateData, amountScaled, 0);
emit LiquidationFinalized(stabilityPool, userAddress, userDebt, getUserCollateralValue(userAddress));
}
StabilityPool::liquidateBorrower:
function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
_update();
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();
bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);
if (!approveSuccess) revert ApprovalFailed();
lendingPool.updateState();
lendingPool.finalizeLiquidation(userAddress);
emit BorrowerLiquidated(userAddress, scaledUserDebt);
}
On the functions above at the highlight marks you can see the points in which the approval and transfer ratios are calculated. The issue which arises is that LendingPool::finalizeLiquidation uses a different way than StabilityPool::liquidateBorrower, LendingPool factors in accrued interest rates, while the StabilityPool does not, resulting in a mismatched (too small) approval within the StabilityPool::liquidateBorrower function, thus reverting the liquidation of the user.
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:
After following above steps copy & paste the following code into ./test/invariant/PoC.t.sol and run forge test --mt test_PocLiquidationNotPossibleDueToAllowanceTransferMismatch -vvvvv
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_PocLiquidationNotPossibleDueToAllowanceTransferMismatch() public {
vm.prank(owner);
lendingPool.setStabilityPool(address(stabilityPool));
crvusd.mint(address(stabilityPool), type(uint128).max);
vm.startPrank(user2);
crvusd.approve(address(lendingPool), type(uint256).max);
lendingPool.deposit(type(uint128).max);
vm.stopPrank();
vm.startPrank(oracle);
raacHousePrices.setHousePrice(1, 10e18);
raacHousePrices.setHousePrice(2, 10e18);
raacHousePrices.setHousePrice(3, 10e18);
raacHousePrices.setHousePrice(4, 10e18);
vm.stopPrank();
vm.startPrank(user1);
crvusd.approve(address(raacNFT), type(uint256).max);
raacNFT.mint(1, 10e18);
raacNFT.mint(2, 10e18);
raacNFT.mint(3, 10e18);
raacNFT.mint(4, 10e18);
raacNFT.approve(address(lendingPool), 1);
raacNFT.approve(address(lendingPool), 2);
raacNFT.approve(address(lendingPool), 3);
lendingPool.depositNFT(1);
lendingPool.depositNFT(2);
lendingPool.depositNFT(3);
lendingPool.borrow(24e18);
vm.startPrank(oracle);
raacHousePrices.setHousePrice(1, 4e18);
raacHousePrices.setHousePrice(2, 4e18);
raacHousePrices.setHousePrice(3, 4e18);
raacHousePrices.setHousePrice(4, 100e18);
vm.stopPrank();
lendingPool.initiateLiquidation(user1);
uint256 time = block.timestamp;
vm.warp(time + 4 * 1 days);
vm.prank(owner);
stabilityPool.liquidateBorrower(user1);
}
Running above test will produce the following log:
Ran 1 test suite in 1.25s (76.97ms CPU time): 0 tests passed, 1 failed, 0 skipped (1 total tests)
Failing tests:
Encountered 1 failing test in test/invariant/PoC.t.sol:PoC
[FAIL: ERC20InsufficientAllowance(0x1d1499e622D69689cdf9004d05Ec547d650Ff211, 24000000000000000000 [2.4e19], 24006576243279862299 [2.4e19])] test_PocLiquidationNotPossibleDueToAllowanceTransferMismatch() (gas: 1943443)
Inspecting the output further with -vvvvv shows us the following 2 lines clearly:
[24780] CrvUSDToken::approve(LendingPool: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], 24000000000000000000 [2.4e19])
emit Approval(owner: StabilityPool: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], spender: LendingPool: [0x1d1499e622D69689cdf9004d05Ec547d650Ff211], value: 24000000000000000000 [2.4e19])
CrvUSDToken::transferFrom(StabilityPool: [0xA4AD4f68d0b91CFD19687c881e50f3A00242828c], RToken: [0x5991A2dF15A8F6A256D3Ec51E99254Cd3fb576A9], 24006576243279862299 [2.4e19])
│ │ │ └─ ← [Revert] ERC20InsufficientAllowance(0x1d1499e622D69689cdf9004d05Ec547d650Ff211, 24000000000000000000 [2.4e19], 24006576243279862299 [2.4e19])
StabilityPool::liquidateBorrower approves 2.4e19 crvUSD for the Lending Pool to pull, but the transfer actually executed carries dust (accrued interest over the 4 days) above the approved balance (2.40065....) causing a revert in the liquidation process.
Impact
Failure to liquidate users leads to accumulation of bad debt and therefor directly harming the solvency invariant of the protocol to the point at which the protocol is not able to pay outstanding balances to it's users. Therefore the severity is High (critical) by default.
Tools Used
Manual Review & Foundry
Recommended Mitigation
Assure that the approval set in StabilityPool::liquidateBorrower is sufficient for the transaction to succeed, by factoring in accrued interest of the borrower, e.g. like:
function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
_update();
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();
- bool approveSuccess = crvUSDToken.approve(address(lendingPool), scaledUserDebt);
+ bool approveSuccess = crvUSDToken.approve(address(lendingPool), type(uint256).max);
if (!approveSuccess) revert ApprovalFailed();
lendingPool.updateState();
lendingPool.finalizeLiquidation(userAddress);
emit BorrowerLiquidated(userAddress, scaledUserDebt);
}
Since the function has strict access control and the lending pool only 1 way to withdraw those funds, a max approval should be okay, to ensure the liquidation will succeed.
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
