DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: high
Valid

checkLiquitableAccounts in the liquidatableBranch.sol will always revert when a lowerbound other than zero is used

Summary

the checkLiquitableAccounts will always revert if the lowerbound is any value other than zero. Meanwhile the lowerBound should be any value between zero and the upperbound.

Vulnerability Details

  • when accounts reaches liquidation.

  • and you try to get the checkLiquidatableAccounts.

  • the upperbound and lowerbound allows you to specify the section of ActiveMarketIds to check.

  • But this function fails when any other lowerbound is used aside 0.

Add this checkLiquitableAccounts.t.sol and run forge test --mc CheckLiquidatableAccounts_Integration_Test --mt test_WhenLowerBoundisNotZero -vvvv

function test_WhenLowerBoundisNotZero( )
external
{
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(1);
uint256 amountOfTradingAccounts = 20;
uint256 marginValueUsd = 10_000e18 / amountOfTradingAccounts;
uint256 initialMarginRate = fuzzMarketConfig.imr;
deal({ token: address(usdz), to: users.naruto.account, give: marginValueUsd });
for (uint256 i = 0; i < amountOfTradingAccounts; i++) {
uint256 accountMarginValueUsd = marginValueUsd / amountOfTradingAccounts;
uint128 tradingAccountId = createAccountAndDeposit(accountMarginValueUsd, address(usdz));
openPosition(fuzzMarketConfig, tradingAccountId, initialMarginRate, accountMarginValueUsd, true);
}
setAccountsAsLiquidatable(fuzzMarketConfig, true);
uint256 lowerBound = 2;
uint256 upperBound = amountOfTradingAccounts;
uint128[] memory liquidatableAccountIds = perpsEngine.checkLiquidatableAccounts(lowerBound, upperBound);
assertEq(liquidatableAccountIds.length, amountOfTradingAccounts);
for (uint256 i; i < liquidatableAccountIds.length; i++) {
// it should return an array with the liquidatable accounts ids
assertEq(liquidatableAccountIds[i], i + 1);
}
}

Impact

This could lead to accounts not being able to be liquidated that's not in the range of lowerbound of 0.

Tools Used

Manual Review

Recommendations

checkLiquidatableAccount function should be reveiwed and the array be better handled

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`LiquidationBranch::checkLiquidatableAccounts()` executes `for` loop where `liquidatableAccountsIds[i] = tradingAccountId;` gives out of bounds if `lowerBound != 0`

Support

FAQs

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