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

Liquidation clears skew and OpenInterest; markPrice will return incorrect data.

alexczm

Summary

When a position is liquidated the skewand openInterestare set to 0 even if there are still open positions.

Vulnerability Details

Protocol has a maxSkew check and a maxOpenInterest check config variables to limit protocol exposure.

Skew is used to calculate the markPrice , price at which new positions are settled.

The problem is that when a position is liquidated both skew and OI are set to 0.

LiquidationBranch.liquidateAccounts() calls perpMarket.updateOpenInterest(); with ctx.newOpenInterestX18 and ctx.newSkewX18 parameters but these parameters are not initialized, meaning that (0, 0) is passed to updateOpenInterest function call.

function liquidateAccounts(uint128[] calldata accountsIds) external {
//...
// update perp market's open interest and skew; we don't enforce ipen
// interest and skew caps during liquidations as:
// 1) open interest and skew are both decreased by liquidations
// 2) we don't want liquidation to be DoS'd in case somehow those cap
// checks would fail
//@audit (0, 0) is passed down the function call
perpMarket.updateOpenInterest(ctx.newOpenInterestX18, ctx.newSkewX18);
}
//...
}
}

Add the folllowing test to test/integration/external/chainlink/keepers/liquidation/performUpkeep/performUpkeep.t.sol file.

First import the following:

import { SD59x18 } from "@prb-math/SD59x18.sol";
import { UD60x18 } from "@prb-math/UD60x18.sol";
import { GlobalConfigurationHarness } from "test/harnesses/perpetuals/leaves/GlobalConfigurationHarness.sol";

Then add the test and execute it with forge test --mt test_LiquidationClears_SkewAndOpenInterest -vvv . The test will pass, all assertions being true.

function test_LiquidationClears_SkewAndOpenInterest(
​
)
external
givenInitializeContract
{
bool isLong = true;
uint256 marketId = 1;
uint256 amountOfTradingAccounts = 1 ;
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(marketId);
uint256 marginValueUsd = 1_000e18;
uint256 initialMarginRate = fuzzMarketConfig.imr;// initial margin rate
deal({ token: address(usdz), to: users.naruto.account, give: marginValueUsd });
​
uint128[] memory accountsIds = new uint128[](2);
​
// create the account that will be liquidated
uint128 tradingAccountId = createAccountAndDeposit(marginValueUsd, address(usdz));
openPosition(fuzzMarketConfig, tradingAccountId, initialMarginRate, marginValueUsd, isLong);
accountsIds[0] = tradingAccountId;
​
//non liquidatable account
deal({ token: address(usdz), to: users.naruto.account, give: marginValueUsd *2 });
uint128 nonLiquidatableTradingAccountId = createAccountAndDeposit(marginValueUsd *2, address(usdz));
openPosition(fuzzMarketConfig, nonLiquidatableTradingAccountId, initialMarginRate, marginValueUsd, isLong);
accountsIds[amountOfTradingAccounts] = nonLiquidatableTradingAccountId;
​
// update collateral price
setAccountsAsLiquidatable(fuzzMarketConfig, isLong);
​
changePrank({ msgSender: users.owner.account });
LiquidationKeeper(liquidationKeeper).setForwarder(users.keepersForwarder.account);
​
changePrank({ msgSender: users.keepersForwarder.account });
bytes memory performData = abi.encode(accountsIds);
​
for (uint256 i; i < accountsIds.length; i++) {
if (accountsIds[i] == nonLiquidatableTradingAccountId) {
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: accountsIds[i],
amountOfOpenPositions: 0,
requiredMaintenanceMarginUsd: 0,
marginBalanceUsd: 0,
liquidatedCollateralUsd: 0,
liquidationFeeUsd: 0
});
}
​
LiquidationKeeper(liquidationKeeper).performUpkeep(performData);
​
// Assertions
assertEq(GlobalConfigurationHarness(address(perpsEngine)).workaround_getAccountsIdsWithActivePositionsLength(),
1,
"Active Positions count is not 1");
assertEq(GlobalConfigurationHarness(address(perpsEngine)).workaround_getAccountIdWithActivePositions(0),
nonLiquidatableTradingAccountId,
"Non-liquidatable trading account id is not correct");
​
SD59x18 skew = perpsEngine.getSkew(uint128(marketId));
assertTrue(skew.intoInt256() == 0, "skew is Not 0");
(UD60x18 longsOpenInterest, UD60x18 shortsOpenInterest, UD60x18 totalOpenInterest) = perpsEngine.getOpenInterest(uint128(marketId));
assertEq(longsOpenInterest.intoUint256(), 0, "longsOpenInterest is not 0");
assertEq(shortsOpenInterest.intoUint256(), 0, "shortsOpenInterest is not 0");
assertEq(totalOpenInterest.intoUint256(), 0, "totalOpenInterest is not 0");
}

Impact

  • maxSkew and maxOpenInterest limits can be exceeded.

  • markPrice for new position (open, close, liquidate) will returns an incorrect price.

Tools Used

Manual review

Recommendations

Calculate the new OI and skek and pass these values to perpMarket.updateOpenInterest. Deduct oldPositionSizeX18 from oldSkew and from oldOI.

Updates

Lead Judging Commences

inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

`liquidateAccounts` calls `updateOpenInterest` with uninitialized OI and skew)

Support

FAQs

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

Give us feedback!