Bug description
The crux of this issue is the fact that during liquidateBorrower(), lendingPool::updateState() is called only after the approval was made, thus resulting in insufficient approval when finalizing liquidation.
StabilityPool.sol#L452-L464
uint256 userDebt = lendingPool.getUserDebt(userAddress);
uint256 scaledUserDebt = WadRayMath.rayMul(
userDebt,
lendingPool.getNormalizedDebt() <-------@ calculating the amount to approve here using non-updated usageIndex, therefore not including interest
);
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(); <---------@ update state here after approval
When calculating user debt, it won't have interest accrued to it, therefore approval will only be made for debt without interest. However, when finalizeLiqudation() will be called it will update reserve state accruing interest to the debt, and will attempt to transfer debt with interest, which will result in revert due to insufficient approval.
LendingPool.sol#L500-L525
ReserveLibrary.updateReserveState(reserve, rateData); <--------@ updating state once more
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, <--------- @here
uint256 newTotalSupply,
uint256 amountBurned,
uint256 balanceIncrease
) = IDebtToken(reserve.reserveDebtTokenAddress).burn(
userAddress,
userDebt,
reserve.usageIndex
);
IERC20(reserve.reserveAssetAddress).safeTransferFrom(
msg.sender,
reserve.reserveRTokenAddress,
amountScaled <----------@ amount transferred includes interest
);
Impact
Liquidations will always revert.
Proof of Concept
Please add import "hardhat/console.sol"; to the top of the StabilityPool.sol and LendingPool.sol.
Modify lending pool's finalizeLiquidation() function.
LendingPool.sol#L522-L523
(
uint256 amountScaled,
uint256 newTotalSupply,
uint256 amountBurned,
uint256 balanceIncrease
) = IDebtToken(reserve.reserveDebtTokenAddress).burn(
userAddress,
userDebt,
reserve.usageIndex
);
// here we log the amount that will be attempted to transfe out
+ console.log("Lending pool debt: ", amountScaled);
Please modify stability pool's liquidateBorrower() function.
StabilityPool.sol#L452-L454
uint256 userDebt = lendingPool.getUserDebt(userAddress);
uint256 scaledUserDebt = WadRayMath.rayMul(
userDebt,
lendingPool.getNormalizedDebt()
);
// here we log the amount that approval will be made for
+ console.log("stabilityPool debt: ", scaledUserDebt);
Please add this test to StabilityPool.test.js and run it with npx hardhat test --grep "liqudiate revert".
describe("sl1", function () {
it("liqudiate revert", async () => {
await raacHousePrices.setHousePrice(1, ethers.parseEther("1000"));
await ethers.provider.send("evm_mine", []);
const tokenId = 1;
const amountToPay = ethers.parseEther("1000");
await crvusd.mint(user1.address, amountToPay);
await crvusd.connect(user1).approve(raacNFT.target, amountToPay);
await raacNFT.connect(user1).mint(tokenId, amountToPay);
await raacNFT.connect(user1).approve(lendingPool.target, tokenId);
await lendingPool.connect(user1).depositNFT(tokenId);
await lendingPool.connect(user1).borrow(ethers.parseEther("1000"));
await lendingPool.initiateLiquidation(user1.address);
await ethers.provider.send("evm_increaseTime", [72 * 60 * 60 + 1]);
await ethers.provider.send("evm_mine");
await crvusd.mint(
await stabilityPool.getAddress(),
ethers.parseEther("1001")
);
await expect(stabilityPool.liquidateBorrower(user1.address)).to.be
.reverted;
});
}
Console output of the test:
stabilityPool debt: 1000000000000000000000
Lending pool debt: 1000462437442040582019
As we can see, the amount of debt approved by the stability pool is greater than the amount of debt that the lending pool is trying to transfer from the stability pool because debt in the stability pool does not have interest accrued to it.
Recommended Mitigation
Call lendingPool::updateState() at the start of the liquidateBorrower() and not at the end.
