Submission Details
Severity:
`LiquidationBranch::checkLiquidatableAccounts` function checks for trading accounts IDs to be liquidated by setting the bounds to be liquidated as (upperBound - lowerBound), this make every check to revert except where lowerbound is zero.
Summary
LiquidationBranch::checkLiquidatableAccounts function checks for trading accounts IDs to be liquidated. The trading accounts IDs to be liquidated as set as (upperBound - lowerBound). This causes an error in the protocol where every check reverts except where the lowerbound is zero.
Vulnerability Details
function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
external
view
returns (uint128[] memory liquidatableAccountsIds)
{
// @audit --> Check
// prepare output array size
@> liquidatableAccountsIds = new uint128[](upperBound - 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;
}
}
}
Poc:
function test_WhenCheckLiquidatableAccountsBreaks()
external
{
MarketConfig memory _marketConfig = marketsConfig[9];
uint256 amountOfTradingAccounts = 10;
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);
}
// ===let's set all the accounts to liquidatable
setAccountsAsLiquidatable(_marketConfig, true);
// let's set check accounts from 5 to 10
uint256 lowerBound = 5;
uint256 upperBound = amountOfTradingAccounts;
vm.expectRevert();
uint128[] memory liquidatableAccountIds = perpsEngine.checkLiquidatableAccounts(lowerBound, upperBound);
}
Impact
This causes an error in the protocol where every check reverts except where the lowerbound is zero. The protocol would not be able to liquidate some accounts that needs to be liquidated by there losing money
Tools Used
Manual
Recommendations
The trading accounts IDs to be liquidated as set as (upperBound)
function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
external
view
returns (uint128[] memory liquidatableAccountsIds)
{
// @audit --> Check
// prepare output array size
- liquidatableAccountsIds = new uint128[](upperBound - 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 11 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.