Submission Details
Severity:
LiquidationBranch::checkLiquidatableAccounts() reverts for lowerbound > 0
Summary
The subtraction upperBound - lowerBound is used to determine the size of the liquidatableAccountsIds array. However, if lowerBound is greater than 0, this causes some accounts within the range of accounts being checked not be liquidated even when they have exceeded the limit for liquidation.
Vulnerability Details
In the SettlementBranch::checkLiquidatableAccounts method, subtracting the upperBound from the lowerBound introduces a vulnerability that could result in some accounts not being liquidated as intended.
function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
external
view
returns (uint128[] memory liquidatableAccountsIds)
{
// prepare output array size
liquidatableAccountsIds = new uint128[](upperBound - lowerBound); //@audit There seems to be a problem with the subtraction of upperbound and lowerbound
// return if nothing to process
if (liquidatableAccountsIds.length == 0) return liquidatableAccountsIds;
// fetch storage slot for global config
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
// cache active account ids length
uint256 cachedAccountsIdsWithActivePositionsLength =
globalConfiguration.accountsIdsWithActivePositions.length();
// iterate over active accounts within given bounds
for (uint256 i = lowerBound; i < upperBound; i++) {
// break if `i` greater then length of active account ids
if (i >= cachedAccountsIdsWithActivePositionsLength) break;
// get the `tradingAccountId` of the current active account
uint128 tradingAccountId = uint128(globalConfiguration.accountsIdsWithActivePositions.at(i));
// load that account's leaf (data + functions)
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(tradingAccountId);
// get that account's required maintenance margin & unrealized PNL
(, UD60x18 requiredMaintenanceMarginUsdX18, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
// get that account's current margin balance
SD59x18 marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
// account can be liquidated if requiredMargin > marginBalance
if (TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) {
liquidatableAccountsIds[i] = tradingAccountId;
}
}
}
Proof Of Code
function test_WhenCheckLiquidatableAccountsBreaks()
external
{
MarketConfig memory _marketConfig = marketsConfig[10];
uint256 amountOfTradingAccounts = 11;
uint256 marginValueUsd = 10_000e18 / amountOfTradingAccounts;
uint256 initialMarginRate = _marketConfig.imr;
deal({ token: address(usdz), to: users.naruto.account, give: marginValueUsd });
for (uint256 i; i < amountOfTradingAccounts; i++) {
uint256 accountMarginValueUsd = marginValueUsd / amountOfTradingAccounts;
uint128 tradingAccountId = createAccountAndDeposit(accountMarginValueUsd, address(usdz));
openPosition(_marketConfig, tradingAccountId, initialMarginRate, accountMarginValueUsd, true);
}
setAccountsAsLiquidatable(_marketConfig, true);
uint256 lowerbound = 5;
uint256 upperbound = amountOfTradingAccounts;
vm.expectRevert();
uint128[] memory liquidatableAccountIds = perpsEngine.checkLiquidatableAccounts(lowerBound, upperBound);
}
When lowerBound is set to 0, the function works correctly. However, if lowerBound is greater than 0, the function consistently returns an "array out of bounds" error.
Impact
Due to this logical error, some accounts may not be liquidated even if they exceed the maximum limit set for liquidation. This oversight can lead to potential losses for the protocol, as it fails to enforce liquidation rules and manage risk effectively.
Tools Used
Foundry
Recommendations
The length of the array liquidatableAccountsids should be the upperbound and not the upperBound - lowerBound.
function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
external
view
returns (uint128[] memory liquidatableAccountsIds)
{
// prepare output array size
- liquidatableAccountsIds = new uint128[](upperBound - lowerBound); //@audit There seems to be a problem with the subtraction of upperbound and lowerbound
+ liquidatableAccountsIds = new uint128[](upperBound);
// return if nothing to process
if (liquidatableAccountsIds.length == 0) return liquidatableAccountsIds;
// fetch storage slot for global config
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
// cache active account ids length
uint256 cachedAccountsIdsWithActivePositionsLength =
globalConfiguration.accountsIdsWithActivePositions.length();
// iterate over active accounts within given bounds
for (uint256 i = lowerBound; i < upperBound; i++) {
// break if `i` greater then length of active account ids
if (i >= cachedAccountsIdsWithActivePositionsLength) break;
// get the `tradingAccountId` of the current active account
uint128 tradingAccountId = uint128(globalConfiguration.accountsIdsWithActivePositions.at(i));
// load that account's leaf (data + functions)
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(tradingAccountId);
// get that account's required maintenance margin & unrealized PNL
(, UD60x18 requiredMaintenanceMarginUsdX18, SD59x18 accountTotalUnrealizedPnlUsdX18) =
tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(0, SD59x18_ZERO);
// get that account's current margin balance
SD59x18 marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18);
// account can be liquidated if requiredMargin > marginBalance
if (TradingAccount.isLiquidatable(requiredMaintenanceMarginUsdX18, marginBalanceUsdX18)) {
liquidatableAccountsIds[i] = tradingAccountId;
}
}
}
Updates
Lead Judging Commences
inallhonesty 10 months ago
Submission Judgement Published Assigned finding tags:
`LiquidationBranch::checkLiquidatableAccounts()` executes `for` loop where `liquidatableAccountsIds[i] = tradingAccountId;` gives out of bounds if `lowerBound != 0`
Prize pool breakdown
Total prize
60,000 USDC
nSLOC
2,87821 USDC / LOC
50,000 USDC
5,500 USDC
4,500 USDC
Live
The contest is live. Earn rewards by submitting a finding.
Appeals
This is your time to appeal against judgements on your submissions.
Appeals Review
Appeals are being carefully reviewed by our judges.