Submission Details
Severity:
Calling the `LiquidationBranch::checkLiquidatableAccounts` function with `lowerBound>upperBound` result in arithmetic underflow or overflow error
Relevant GitHub Links
Summary
In the LiquidationBranch::checkLiquidatableAccounts function which expects 2 parameters, if the lowerBound param is called with a value greater than the upperBound param, a negative value is output.
Vulnerability Details
In the upperBound - lowerBound operation of the following function, 2 numbers of type uint256 are subtracted, so if lowerBound>upperBound, the result will be negative and, in our case, we will have a negative array, which makes no sense.
function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
external
view
returns (uint128[] memory liquidatableAccountsIds)
{
// 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;
}
}
}
Impact
We will not be able to obtain a table of accounts because of the arithmetic underflow or overflow error
Tools Used
Fuzz test
Recommendations
We should check that upperBound>lowerBound before the substraction has follow
function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
external
view
returns (uint128[] memory liquidatableAccountsIds)
{
+ require(upperBound > lowerBound, 'Upper must be greater than Lower!');
// 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;
}
}
}
Updates
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.