Submission Details
Severity:
A malicious user can grief a `LiquidationBranch::checkLiquidatableAccounts` contract by setting huge difference between `upperBound` and `lowerBound` of the accounts to check
Summary
The LiquidationBranch::checkLiquidatableAccounts function iterates over a range of account IDs specified by lowerBound and upperBound parameters to identify liquidatable accounts. This function does not impose a limit on the difference between upperBound and lowerBound, allowing for an excessively large range to be specified.
Vulnerability Details
A malicious actor could exploit this lack of constraint by setting a huge difference between upperBound and lowerBound (significantly higher than necessary), forcing the function to iterate over a much larger range than intended. This excessive iteration could lead to gas exhaustion.
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;
... //omitted code
// 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;
... //omitted code
}
}
}
Impact
When a function processes an extensive list, there's a risk of it consuming all available gas. Consequently, it fails, throwing an out-of-gas exception, which negatively affects users trying to interact with the contract.
POC
Copy and paste this test in liquisateAccount.t.sol
Run: forge test --match-test testFuzz_GivenThereAreLiquidatableAccountsInTheHugeArray -vvvv
function testFuzz_GivenThereAreLiquidatableAccountsInTheHugeArray(
uint256 marketId,
uint256 secondMarketId,
bool isLong,
uint256 timeDelta
)
external
givenTheSenderIsARegisteredLiquidator
whenTheAccountsIdsArrayIsNotEmpty
givenAllAccountsExist
{
uint256 initialGasLeft = gasleft();
console2.log("initialGasLeft", initialGasLeft);
TestFuzz_GivenThereAreLiquidatableAccountsInTheArray_Context memory ctx;
ctx.fuzzMarketConfig = getFuzzMarketConfig(marketId);
ctx.secondMarketConfig = getFuzzMarketConfig(secondMarketId);
vm.assume(ctx.fuzzMarketConfig.marketId != ctx.secondMarketConfig.marketId);
// ** amount of trading accounts to change for the test ** //
uint256 amountOfTradingAccounts = 10;
timeDelta = bound({ x: timeDelta, min: 1 seconds, max: 1 days });
ctx.marginValueUsd = 1_000_000e18 / amountOfTradingAccounts;
ctx.initialMarginRate = ctx.fuzzMarketConfig.imr;
deal({ token: address(usdz), to: users.naruto.account, give: ctx.marginValueUsd });
// last account id == 0
ctx.accountsIds = new uint128[](amountOfTradingAccounts + 2);
ctx.accountMarginValueUsd = ctx.marginValueUsd / (amountOfTradingAccounts + 1);
for (uint256 i; i < amountOfTradingAccounts; i++) {
ctx.tradingAccountId = createAccountAndDeposit(ctx.accountMarginValueUsd, address(usdz));
openPosition(
ctx.fuzzMarketConfig,
ctx.tradingAccountId,
ctx.initialMarginRate,
ctx.accountMarginValueUsd / 2,
isLong
);
openPosition(
ctx.secondMarketConfig,
ctx.tradingAccountId,
ctx.secondMarketConfig.imr,
ctx.accountMarginValueUsd / 2,
isLong
);
ctx.accountsIds[i] = ctx.tradingAccountId;
deal({ token: address(usdz), to: users.naruto.account, give: ctx.marginValueUsd });
}
setAccountsAsLiquidatable(ctx.fuzzMarketConfig, isLong);
setAccountsAsLiquidatable(ctx.secondMarketConfig, isLong);
ctx.nonLiquidatableTradingAccountId = createAccountAndDeposit(ctx.accountMarginValueUsd, address(usdz));
ctx.accountsIds[amountOfTradingAccounts] = ctx.nonLiquidatableTradingAccountId;
changePrank({ msgSender: liquidationKeeper });
for (uint256 i; i < ctx.accountsIds.length; i++) {
if (ctx.accountsIds[i] == ctx.nonLiquidatableTradingAccountId || ctx.accountsIds[i] == 0) {
continue;
}
// it should emit a {LogLiquidateAccount} event
vm.expectEmit({
checkTopic1: true,
checkTopic2: true,
checkTopic3: false,
checkData: false,
emitter: address(perpsEngine)
});
emit LiquidationBranch.LogLiquidateAccount({
keeper: liquidationKeeper,
tradingAccountId: ctx.accountsIds[i],
amountOfOpenPositions: 0,
requiredMaintenanceMarginUsd: 0,
marginBalanceUsd: 0,
liquidatedCollateralUsd: 0,
liquidationFeeUsd: 0
});
}
skip(timeDelta);
ctx.expectedLastFundingRate = perpsEngine.getFundingRate(ctx.fuzzMarketConfig.marketId).intoInt256();
ctx.expectedLastFundingTime = block.timestamp;
perpsEngine.liquidateAccounts(ctx.accountsIds);
// it should update the market's funding values
ctx.perpMarketData =
PerpMarketHarness(address(perpsEngine)).exposed_PerpMarket_load(ctx.fuzzMarketConfig.marketId);
assertEq(ctx.expectedLastFundingRate, ctx.perpMarketData.lastFundingRate, "last funding rate");
assertEq(ctx.expectedLastFundingTime, ctx.perpMarketData.lastFundingTime, "last funding time");
// it should update open interest value
(,, ctx.openInterestX18) = perpsEngine.getOpenInterest(ctx.fuzzMarketConfig.marketId);
ctx.expectedOpenInterest = sd59x18(
PositionHarness(address(perpsEngine)).exposed_Position_load(
ctx.nonLiquidatableTradingAccountId, ctx.fuzzMarketConfig.marketId
).size
).abs().intoUD60x18().intoUint256();
assertEq(ctx.expectedOpenInterest, ctx.openInterestX18.intoUint256(), "open interest");
// it should update skew value
ctx.skewX18 = perpsEngine.getSkew(ctx.fuzzMarketConfig.marketId);
ctx.expectedSkew = PositionHarness(address(perpsEngine)).exposed_Position_load(
ctx.nonLiquidatableTradingAccountId, ctx.fuzzMarketConfig.marketId
).size;
assertEq(ctx.expectedSkew, ctx.skewX18.intoInt256(), "skew");
for (uint256 i; i < ctx.accountsIds.length; i++) {
if (ctx.accountsIds[i] == ctx.nonLiquidatableTradingAccountId) {
continue;
}
// it should delete any active market order
ctx.marketOrder = perpsEngine.getActiveMarketOrder(ctx.accountsIds[i]);
assertEq(ctx.marketOrder.marketId, 0);
assertEq(ctx.marketOrder.sizeDelta, 0);
assertEq(ctx.marketOrder.timestamp, 0);
// it should close all active positions
ctx.expectedPosition =
Position.Data({ size: 0, lastInteractionPrice: 0, lastInteractionFundingFeePerUnit: 0 });
ctx.position = PositionHarness(address(perpsEngine)).exposed_Position_load(
ctx.accountsIds[i], ctx.fuzzMarketConfig.marketId
);
assertEq(ctx.expectedPosition.size, ctx.position.size, "position size");
assertEq(ctx.expectedPosition.lastInteractionPrice, ctx.position.lastInteractionPrice, "position price");
assertEq(
ctx.expectedPosition.lastInteractionFundingFeePerUnit,
ctx.position.lastInteractionFundingFeePerUnit,
"position funding fee"
);
// it should remove the account's all active markets
assertEq(
0,
TradingAccountHarness(address(perpsEngine)).workaround_getActiveMarketsIdsLength(ctx.accountsIds[i]),
"active market id"
);
assertEq(
0,
GlobalConfigurationHarness(address(perpsEngine)).workaround_getAccountsIdsWithActivePositionsLength(),
"accounts ids with active positions"
);
}
uint256 finalGasLeft = gasleft();
console2.log("finalGasLeft", finalGasLeft);
uint256 gasUsed = initialGasLeft - finalGasLeft;
console2.log("gas used", gasUsed);
// block gaslimit for Arbitrum
uint256 GAS_LIMIT = 32_000_000 wei;
assert(gasUsed < GAS_LIMIT);
}
The test passes.
Log:
├─ \[0] console::log("gas used", 17488899 \[1.748e7]) \[staticcall]
│ └─ ← \[Stop]
└─ ← \[Stop]
Suite result: ok. 1 passed; 0 failed; 0 skipped; finished in 32.41s (32.39s CPU time)
Now run the same test setting the amount of trading accounts to 20:
uint256 amountOfTradingAccounts = 20; //line 23
The test fails:
Logs:
├─ [0] console::log("gas used", 33615062 [3.361e7]) [staticcall]
│ └─ ← [Stop]
└─ ← [Revert] panic: assertion failed (0x01)
Suite result: FAILED. 0 passed; 1 failed; 0 skipped; finished in 89.70ms (72.52ms CPU time)
Tools Used
Manual review.
Recommendations
Introduce a maximum limit for the difference between upperBound and lowerBound to prevent excessively large ranges from being processed.
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.