Zaros Part 1
Zaros
DeFiFoundry
60,000 USDC
View results
Previous Next
Submission Details
Severity: high
Invalid

Unrestricted Iteration Count (User-Provided Bound + Potential DoS)

OdeWeb3

Summary

The function checkLiquidatableAccounts can be exploited by providing a high upperBound, causing excessive iterations and leading to potential denial of service (DoS) attacks.

Vulnerability Details

Description:

  • The upperBound parameter is controlled by the user and dictates the number of iterations the function performs.

  • A malicious user can set a very high upperBound, forcing the function to perform excessive computations.

Root Cause:

  • Lack of validation and limitation on the upperBound parameter to prevent excessive iterations.

Proof of Concept:

Copy and paste this in checkLiquidatableAccounts.t.sol P.S: increase the user fund.

function test_WhenUpperBoundExceedsMaxAccountsToCheck2() external {
// create some accounts first
createAccounts(150);
​
uint256 lowerBound = 0;
uint256 upperBound = 150;
​
uint128[] memory liquidatableAccountIds = perpsEngine.checkLiquidatableAccounts(lowerBound, upperBound);
​
// it should process only up to the maximum allowed number of accounts
assertEq(liquidatableAccountIds.length, MAX_ACCOUNTS_TO_CHECK);
}
// helper function to create accounts
function createAccounts(uint256 amountOfTradingAccounts) internal {
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(1);
uint256 initialMarginRate = fuzzMarketConfig.imr;
for (uint256 i; i < amountOfTradingAccounts; i++) {
uint256 accountMarginValueUsd = 10_000e18;
uint128 tradingAccountId = createAccountAndDeposit(accountMarginValueUsd, address(usdz));
openPosition(fuzzMarketConfig, tradingAccountId, initialMarginRate, accountMarginValueUsd, true);
}
}

Impact

  • Potential denial of service (DoS) or griefing attack by exhausting computational resources.

  • Degradation of system performance and potential unavailability of services.

Tools Used

Manual review

Recommendations

// Maximum number of accounts to check in a single call
uint256 constant MAX_ACCOUNTS_TO_CHECK = 100;
​
/// @param lowerBound The lower bound of the accounts to check
/// @param upperBound The upper bound of the accounts to check
function checkLiquidatableAccounts(
uint256 lowerBound,
uint256 upperBound
)
external
view
returns (uint128[] memory liquidatableAccountsIds)
{
// fetch storage slot for global config
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
​
// cache active account ids length
uint256 cachedAccountsIdsWithActivePositionsLength =
globalConfiguration.accountsIdsWithActivePositions.length();
​
// if lowerBound is greater than the length of active accounts, return an empty array
if (lowerBound >= cachedAccountsIdsWithActivePositionsLength) {
return new uint128 ;
}
​
// adjust upperBound to not exceed the length of active accounts
if (upperBound > cachedAccountsIdsWithActivePositionsLength) {
upperBound = cachedAccountsIdsWithActivePositionsLength;
}
​
// Limit the number of accounts checked to MAX_ACCOUNTS_TO_CHECK
if (upperBound - lowerBound > MAX_ACCOUNTS_TO_CHECK) {
upperBound = lowerBound + MAX_ACCOUNTS_TO_CHECK;
}
​
// prepare output array size
liquidatableAccountsIds = new uint128[](upperBound - lowerBound);
​
// iterate over active accounts within given bounds
for (uint256 i = lowerBound; i < upperBound; i++) {
// 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 - lowerBound] = tradingAccountId;
}
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

Can't find an answer? Chat with us on Discord, Twitter or Linkedin.