Submission Details
Severity:
Oversight in `LiquidationBranch::liquidateAccounts` function causes the related perp market skew and open interest to be incorrectly updated
Summary
The perp market skew and open interest are incorrectly updated after a liquidation.
Vulnerability Details
Whenever the size delta of a position is affected, the protocol is updated to reflect the change in the associated market's skew and open interest. This happens when an order is filled and when an account is liquidated.
In case of order filling, the skew and open interest update is handled correctly, as seen here.
However, the case is different for liquidation. The open interest update is handled incorrectly, as the ctx.newOpenInterestX18 and ctx.newSkewX18 are never set. The ctx struct variable was declared here in memory, but its newOpenInterestX18 and newSkewX18 were never used or set until the point of skew and open interest update.
function updateOpenInterest(Data storage self, UD60x18 newOpenInterest, SD59x18 newSkew) internal {
self.skew = newSkew.intoInt256().toInt128();
self.openInterest = newOpenInterest.intoUint128();
}
The above code snippet shows how the PerpMarket::updateOpenInterest function works. It replaces the existing skew and open interest of the market with the corresponding arguments. Which means after liquidation, the skew and open Inteerest of the associated market is reset to zero, irrespective of where the market still has other active positions.
Proof of Code
The PoC below shows that the skew and open interest of a market after liquidation is reset to zero despite having other active positions in the market.
How to run the PoC
Create a new test file named
SubmissionPoC.t.solintest/folder of the contest repoCopy the code below and paste in the file.
Run the commands above the test functions in your CLI to run the test.
<details>
// SPDX-License-Identifier: UNLICENSED
// Zaros dependencies
import { Base\_Test } from "test/Base.t.sol";
import { Errors } from "@zaros/utils/Errors.sol";
import { ERC20, IERC20 } from "@openzeppelin/token/ERC20/ERC20.sol";
import {console2} from "forge-std/console2.sol";
import { MockERC20 } from "test/mocks/MockERC20.sol";
import { MockPriceFeed } from "test/mocks/MockPriceFeed.sol";
import { Users, User, MockPriceAdapters } from "test/utils/Types.sol";
import { UpgradeBranch } from "@zaros/tree-proxy/branches/UpgradeBranch.sol";
import { LookupBranch } from "@zaros/tree-proxy/branches/LookupBranch.sol";
import { GlobalConfigurationBranch } from "@zaros/perpetuals/branches/GlobalConfigurationBranch.sol";
import { LiquidationBranch } from "@zaros/perpetuals/branches/LiquidationBranch.sol";
import { OrderBranch } from "@zaros/perpetuals/branches/OrderBranch.sol";
import { PerpMarketBranch } from "@zaros/perpetuals/branches/PerpMarketBranch.sol";
import { TradingAccountBranch } from "@zaros/perpetuals/branches/TradingAccountBranch.sol";
import { SettlementBranch } from "@zaros/perpetuals/branches/SettlementBranch.sol";
import { MarginCollateralConfiguration } from "@zaros/perpetuals/leaves/MarginCollateralConfiguration.sol";
import { Position } from "@zaros/perpetuals/leaves/Position.sol";
import { MarketOrder } from "@zaros/perpetuals/leaves/MarketOrder.sol";
import { SettlementConfiguration } from "@zaros/perpetuals/leaves/SettlementConfiguration.sol";
import { TradingAccountBranchTestnet } from "testnet/branches/TradingAccountBranchTestnet.sol";
import { GlobalConfigurationHarness } from "test/harnesses/perpetuals/leaves/GlobalConfigurationHarness.sol";
import { MarginCollateralConfigurationHarness } from "test/harnesses/perpetuals/leaves/MarginCollateralConfigurationHarness.sol";
import { MarketConfigurationHarness } from "test/harnesses/perpetuals/leaves/MarketConfigurationHarness.sol";
import { MarketOrderHarness } from "test/harnesses/perpetuals/leaves/MarketOrderHarness.sol";
import { PerpMarketHarness } from "test/harnesses/perpetuals/leaves/PerpMarketHarness.sol";
import { PositionHarness } from "test/harnesses/perpetuals/leaves/PositionHarness.sol";
import { SettlementConfigurationHarness } from "test/harnesses/perpetuals/leaves/SettlementConfigurationHarness.sol";
import { TradingAccountHarness } from "test/harnesses/perpetuals/leaves/TradingAccountHarness.sol";
import {LiquidationKeeper} from "@zaros/external/chainlink/keepers/liquidation/LiquidationKeeper.sol";
import {MarketOrderKeeper} from "@zaros/external/chainlink/keepers/market-order/MarketOrderKeeper.sol";
// PRB Math dependencies
import { UD60x18, ud60x18, ZERO as UD60x18\_ZERO, convert as ud60x18Convert } from "@prb-math/UD60x18.sol";
import {SD59x18,sd59x18, unary, UNIT as SD\_UNIT, ZERO as SD59x18\_ZERO, convert as sd59x18Convert} from "@prb-math/SD59x18.sol";
contract BugPoC is Base\_Test {
```Solidity
///////////// SETUP //////////////
address[] internal USERS;
MockERC20[] internal EXTERNAL_TOKENS;
address internal BOB;
address internal ALICE;
address internal defaultUser; // defaultUser
address USDC;
address wBTC;
address weETH;
uint256 internal constant INITIAL_USDC_BALANCE = 1_000_000;
uint256 internal constant INITIAL_WEETH_BALANCE = 100_000;
uint256 internal constant INITIAL_WBTC_BALANCE = 10_000;
MockPriceFeed btcPriceFeed;
MockPriceFeed ethPriceFeed;
function setUp() public override {
Base_Test.setUp();
changePrank({ msgSender: users.owner.account });
configureSystemParameters();
createPerpMarkets();
EXTERNAL_TOKENS = new MockERC20[](3);
EXTERNAL_TOKENS[0] = usdc;
EXTERNAL_TOKENS[1] = weEth;
EXTERNAL_TOKENS[2] = wBtc;
USDC = address(usdc);
wBTC = address(wBtc);
weETH = address(weEth);
btcPriceFeed = MockPriceFeed(marketsConfig[1].priceAdapter);
ethPriceFeed = MockPriceFeed(marketsConfig[2].priceAdapter);
BOB = users.naruto.account;
ALICE = users.sasuke.account;
USERS = new address[](2);
USERS[0] = BOB;
USERS[1] = ALICE;
_topUpUsers();
vm.stopPrank();
}
function _topUpUsers() internal {
address user;
for (uint8 i = 0; i < USERS.length; i++) {
user = USERS[i];
EXTERNAL_TOKENS[0].mint(user, INITIAL_USDC_BALANCE * (10 ** EXTERNAL_TOKENS[0].decimals()));
EXTERNAL_TOKENS[1].mint(user, INITIAL_WEETH_BALANCE * (10 ** EXTERNAL_TOKENS[0].decimals()));
EXTERNAL_TOKENS[2].mint(user, INITIAL_WBTC_BALANCE * (10 ** EXTERNAL_TOKENS[0].decimals()));
}
}
function clampBetween(
uint256 value,
uint256 low,
uint256 high
) internal pure returns (uint256) {
if (value < low || value > high) {
uint ans = low + (value % (high - low + 1));
return ans;
}
return value;
}
modifier updateMockPriceFeedBtcEth(uint256 _btcNewPrice, uint256 _ethNewPrice) {
uint256 btcNewPrice = clampBetween(_btcNewPrice, 10_000e18, 100_000e18);
uint256 ethNewPrice = clampBetween(_ethNewPrice, 500e18, 10_000e18);
updateMockPriceFeed(1, btcNewPrice);
updateMockPriceFeed(2, ethNewPrice);
_;
}
function getPriceUint(MockPriceFeed priceFeed) internal view returns (uint256) {
(, int256 answer,,,) = priceFeed.latestRoundData();
return uint256(answer);
}
function _performLiquidation(uint128 nextAccountId) internal {
LiquidationKeeper liquidationKeeper_ = LiquidationKeeper(liquidationKeeper);
uint256 checkLowerBound = 0;
uint256 checkUpperBound = uint256(nextAccountId);
uint256 performLowerBound = 0;
uint256 performUpperBound = checkUpperBound + 1;
if (checkUpperBound < 1) return;
bytes memory data = abi.encode(checkLowerBound, checkUpperBound, performLowerBound, performUpperBound);
(bool upkeepNeeded, bytes memory performData) = liquidationKeeper_.checkUpkeep(data);
vm.startPrank(users.owner.account);
liquidationKeeper_.setForwarder(users.keepersForwarder.account);
if (upkeepNeeded) {
vm.startPrank(users.keepersForwarder.account);
liquidationKeeper_.performUpkeep(performData);
vm.stopPrank();
}
}
struct TestHelper {
uint256 oldBtcPrice;
uint256 oldEthPrice;
uint256 btcNewPrice;
uint256 ethNewPrice;
uint256 price;
bytes32 streamId;
uint256 priceBtcChange;
uint256 priceEthChange;
OrderBranch.CreateMarketOrderParams params;
}
function _performOrderExecution(uint256 marketId, uint128 tradingAccountId, bool isIncrease) internal {
TestHelper memory helper;
helper.oldBtcPrice = getPriceUint(btcPriceFeed);
helper.oldEthPrice = getPriceUint(ethPriceFeed);
helper.btcNewPrice;
helper.ethNewPrice;
helper.price;
helper.streamId = marketsConfig[marketId].streamId;
helper.priceBtcChange = helper.oldBtcPrice % 100e18;
helper.priceEthChange = helper.oldEthPrice % 10e18;
if(isIncrease) {
helper.btcNewPrice = helper.oldBtcPrice + helper.priceBtcChange;
helper.ethNewPrice = helper.oldEthPrice + helper.priceEthChange;
} else {
helper.btcNewPrice = helper.oldBtcPrice - helper.priceBtcChange;
helper.ethNewPrice = helper.oldEthPrice - helper.priceEthChange;
}
btcPriceFeed.updateMockPrice(helper.btcNewPrice);
ethPriceFeed.updateMockPrice(helper.ethNewPrice);
address marketOrderKeeper_ = marketOrderKeepers[marketId];
MarketOrderKeeper marketOrderKeeper = MarketOrderKeeper(marketOrderKeeper_);
if (marketId == 1) {
helper.price = getPriceUint(btcPriceFeed);
} else {
helper.price = getPriceUint(ethPriceFeed);
}
bytes memory mockSignedReport = getMockedSignedReport(helper.streamId, helper.price);
vm.startPrank(users.owner.account);
marketOrderKeeper.setForwarder(users.keepersForwarder.account);
vm.stopPrank();
bytes memory performData = abi.encode(mockSignedReport, abi.encode(tradingAccountId));
vm.startPrank(users.keepersForwarder.account);
marketOrderKeeper.performUpkeep(performData);
vm.stopPrank();
}
function tradingAccountBranch_createTradingAccount(
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) returns(uint128 _tradingAccountId){
return perpsEngine.createTradingAccount(bytes(""), false) ;
}
/// depositMargin
function tradingAccountBranch_depositMargin(
uint128 _tradingAccountId,
address collateralType,
uint256 _amount,
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) {
perpsEngine.depositMargin(_tradingAccountId, collateralType, _amount);
}
/// withdrawMargin
function tradingAccountBranch_withdrawMargin(
uint128 _tradingAccountId,
address collateralType,
uint256 _amount,
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) {
perpsEngine.withdrawMargin(_tradingAccountId, collateralType, _amount);
}
/// createMarketOrder
function orderBranch_createMarketOrder(
uint128 _tradingAccountId,
bool _buyBtc,
int128 sizeDelta,
bool isIncrease,
uint256 _btcNewPrice,
uint256 _ethNewPrice
) public updateMockPriceFeedBtcEth(_btcNewPrice, _ethNewPrice) {
TestHelper memory helper;
helper.params.tradingAccountId = _tradingAccountId;
helper.params.marketId = _buyBtc ? 1 : 2;
helper.params.sizeDelta = sizeDelta;
perpsEngine.createMarketOrder(helper.params);
_performOrderExecution(helper.params.marketId, helper.params.tradingAccountId, isIncrease);
}
///////////// TEST //////////////
////////////////// Test Skew and OI Update ////////////////
// forge test --match-test "testSkewAndOIUpdate"
function testSkewAndOIUpdate() public {
vm.warp(1 hours);
vm.startPrank(BOB);
// BOB creates a trading account
tradingAccountBranch_createTradingAccount(30_000e18, 2_000e18);
uint256 activeMarketId_0 = perpsEngine.workaround_getActiveMarketsIdsLength(1);
console2.log("activeMarketIdLength 0: ", activeMarketId_0); // should be 0
vm.startPrank(BOB);
// BOB deposit 10000e6 USDC
tradingAccountBranch_depositMargin(1, USDC, 10000e6, 100_000e18, 10_000e18);
vm.startPrank(BOB);
// BOB creates market order of 1e18 BTC @ 100_000e18 USDC : 10x leverage
// the orderBranch_createMarketOrder handles the order execution
orderBranch_createMarketOrder(1, true, 1e18, true, 100_000e18, 10_000e18);
uint256 activeMarketId_1 = perpsEngine.workaround_getActiveMarketsIdsLength(1); // should be 1
console2.log("activeMarketIdLength 1: ", activeMarketId_1);
// check how many markets are active in the system
uint256 activeMarketId0 = perpsEngine.workaround_getActiveMarketsIdsLength(1);
console2.log("activeMarketIdLength 1: ", activeMarketId0); // should be 1
// check the market skew
SD59x18 currentSkew0 = perpsEngine.getSkew(1);
// check the market open interest
(,, UD60x18 totalOpenInterest0) = perpsEngine.getOpenInterest(1);
console2.log("currentSkew 1: ", currentSkew0.intoUD60x18().intoUint256()); // Log:
console2.log("totalOpenInterest 1: ", totalOpenInterest0.intoUint256()); // Log:
vm.startPrank(ALICE);
// Another user enters the market
tradingAccountBranch_createTradingAccount(100_000e18, 10_000e18);
// create trading account after updating price to 1_000e18 BTC and 100e18 ETH
// the significant price drop will lead to liquidation
vm.startPrank(ALICE);
// deposit 10000e6 USDC
tradingAccountBranch_depositMargin(2, USDC, 10000e6, 100_000e18, 10_000e18);
vm.startPrank(ALICE);
// create Market order of 1e18 BTC @ 1_000e18 USDC : 10x leverage
// the price drop will lead to liquidation of BOBs accounts
orderBranch_createMarketOrder(2, true, 1e18, true, 1_000e18, 100e18);
// check the market skew before liquidation
SD59x18 currentSkew1 = perpsEngine.getSkew(1);
// check the market open interest before liquidation
(,, UD60x18 totalOpenInterest1) = perpsEngine.getOpenInterest(1);
console2.log("currentSkew 2: ", currentSkew1.intoUD60x18().intoUint256()); // Log:
console2.log("totalOpenInterest 2: ", totalOpenInterest1.intoUint256()); // Log:
// check if BOB is liquidatable
(SD59x18 marginBalanceUsdX18, , UD60x18 requiredMaintenanceMarginUsdX18, ) = perpsEngine.getAccountMarginBreakdown(1);
bool liquidatable = perpsEngine.exposed_isLiquidatable(requiredMaintenanceMarginUsdX18, marginBalanceUsdX18);
console2.log("liquidatable: ", liquidatable); // This log true
// check if ALICE is liquidatable
(SD59x18 marginBalanceUsdX18_alice, , UD60x18 requiredMaintenanceMarginUsdX18_alice, ) = perpsEngine.getAccountMarginBreakdown(2);
bool liquidatable2 = perpsEngine.exposed_isLiquidatable(requiredMaintenanceMarginUsdX18_alice, marginBalanceUsdX18_alice);
console2.log("liquidatable2: ", liquidatable2); // This logs false
// perform liquidation
_performLiquidation(2); // this function takes the latest trading account id as argument
// check the market skew
SD59x18 currentSkew2 = perpsEngine.getSkew(1);
// check the market open interest
(,, UD60x18 totalOpenInterest2) = perpsEngine.getOpenInterest(1);
console2.log("currentSkew 3: ", currentSkew2.intoUD60x18().intoUint256()); // Log: 0
console2.log("totalOpenInterest 3: ", totalOpenInterest2.intoUint256()); // Log: 0
// Log:
// activeMarketIdLength 0: 0
// activeMarketIdLength 1: 1
// activeMarketIdLength 1: 1
// currentSkew 1: 1000000000000000000
// totalOpenInterest 1: 1000000000000000000
// currentSkew 2: 2000000000000000000
// totalOpenInterest 2: 2000000000000000000
// liquidatable: true
// liquidatable2: false
// currentSkew 3: 0
// totalOpenInterest 3: 0
// Note that despite Alices account not being liquidatable, she is liquidated along with BOB because the market is wiped clean
// after any liquidation.
}
</details>
Impact
The bad update of skew and open interest of the protocol can cause a wide range of problems in the protocol ranging from asset mispricing, incorrect fee calculations, break of the maximum and minimum constraint, and general loss of value for the market and trading accounts.
Tools Used
Manual
Recommendations
Calculate the right values and set them to ctx.newOpenInterestX18 and ctx.newSkewX18, before passing them in PerpMarket::updateOpenInterest function when called.
Updates
Lead Judging Commences
inallhonesty 11 months ago
Submission Judgement Published Assigned finding tags:
`liquidateAccounts` calls `updateOpenInterest` with uninitialized OI and skew)
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.