Summary
StabilityPool's liquidateBorrower function can revert as it uses LendingPool's state for some checks and it updates it after.
Vulnerability Details
This is the liquidateBorrower function:
StabilityPool.sol
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);
}
As we can see, it first calls lendingPool.getNormalizedDebt() which gets the usageIndex of the LendingPool. It then uses it to calculate scaledUserDebt, which is used in a check that crvUSDBalance < scaledUserDebt. At last, it calls lendingPool.updateState() which updates the liquidityIndex and usageIndex of the LendingPool. Therefore, the usageIndex used to calculate the scaledUserDebt may be outdated.
Later, in the LendingPool's finalizeLiquidation function, the updated usageIndex is used to calculate the user's debt:
LendingPool.sol
function finalizeLiquidation(address userAddress) external nonReentrant onlyStabilityPool {
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);
That means that if an outdated usageIndex is used at first and then the usageIndex is bigger than the previous one, the if (crvUSDBalance < scaledUserDebt) revert InsufficientBalance(); check might not actually be true. The finalizeLiquidation will attempt to transfer a bigger amount of tokens than the one approved in StabilityPool.
Impact
The liquidateBorrower function will revert and prevent the manager or owner from liquidating a user.
Tools Used
Manual review
Recommendations
Update the ReservePool's state first and then use the usageIndex:
function liquidateBorrower(address userAddress) external onlyManagerOrOwner nonReentrant whenNotPaused {
_update();
++ lendingPool.updateState();
// 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);
}
