Summary
The current implementation calculates the maintenance margin based on the current market price rather than the initial position price.
As a result, when the price of an asset decreases, the maintenance margin requirement for existing positions also decreases.
Vulnerability Details
The root cause is in the TradingAccount::getAccountMarginRequirementUsdAndUnrealizedPnlUsd function, where the notional value and subsequent margin calculations use the current mark price instead of the initial position price. This allows traders to maintain positions with insufficient collateral, potentially leading to undercollateralized positions and increased systemic risk.
function getAccountMarginRequirementUsdAndUnrealizedPnlUsd(
Data storage self,
uint128 targetMarketId,
SD59x18 sizeDeltaX18
)
internal
view
returns (
UD60x18 requiredInitialMarginUsdX18,
UD60x18 requiredMaintenanceMarginUsdX18,
SD59x18 accountTotalUnrealizedPnlUsdX18
)
{
if (targetMarketId != 0) {
@> UD60x18 markPrice = perpMarket.getMarkPrice(sizeDeltaX18, perpMarket.getIndexPrice());
@> UD60x18 notionalValueX18 = sd59x18(position.size).add(sizeDeltaX18).abs().intoUD60x18().mul(markPrice);
(UD60x18 positionInitialMarginUsdX18, UD60x18 positionMaintenanceMarginUsdX18) = Position
.getMarginRequirement(
@> notionalValueX18,
ud60x18(perpMarket.configuration.initialMarginRateX18),
ud60x18(perpMarket.configuration.maintenanceMarginRateX18)
);
Impact
The impact of this vulnerability is severe:
Increased risk of protocol insolvency due to undercollateralized positions and compromised risk management mechanisms.
Unfair advantage to traders with losing positions, enabling them to avoid liquidation and potentially manipulate the market with highly leveraged positions and minimal collateral.
Potential domino effect in extreme market conditions, leading to failed liquidations and significant losses for the protocol.
Proof of Concept
-
A trader opens a long position of 1 BTC at $10,000, with a 0.5% maintenance margin rate.
-
The price drops to $1,000.
-
Current implementation calculates new maintenance margin: $5 (0.5% of $1,000)
-
The position becomes undercollateralized relative to its initial risk, but is not liquidated due to the reduced margin requirement.
Proof of Code
First, add this helper function in test/Base.t.sol. We are going to set the order et settlment fees to 0 in order to simplify the calculations of the margin:
function setFeesTo0(uint128 marketId) internal {
GlobalConfigurationBranch.UpdatePerpMarketConfigurationParams memory params = GlobalConfigurationBranch
.UpdatePerpMarketConfigurationParams({
name: marketsConfig[marketId].marketName,
symbol: marketsConfig[marketId].marketSymbol,
priceAdapter: marketsConfig[marketId].priceAdapter,
initialMarginRateX18: marketsConfig[marketId].imr,
maintenanceMarginRateX18: marketsConfig[marketId].mmr,
maxOpenInterest: marketsConfig[marketId].maxOi,
maxSkew: marketsConfig[marketId].maxSkew,
maxFundingVelocity: marketsConfig[marketId].maxFundingVelocity,
skewScale: marketsConfig[marketId].skewScale,
minTradeSizeX18: marketsConfig[marketId].minTradeSize,
orderFees: OrderFees.Data({makerFee: 0, takerFee: 0}),
priceFeedHeartbeatSeconds: marketsConfig[marketId].priceFeedHeartbeatSeconds
});
perpsEngine.updatePerpMarketConfiguration(marketId, params);
SettlementConfiguration.DataStreamsStrategy memory marketOrderConfigurationData = SettlementConfiguration
.DataStreamsStrategy({
chainlinkVerifier: IVerifierProxy(mockChainlinkVerifier),
streamId: marketsConfig[marketId].streamId
});
SettlementConfiguration.Data memory newSettlementConfiguration = SettlementConfiguration.Data({
strategy: SettlementConfiguration.Strategy.DATA_STREAMS_DEFAULT,
isEnabled: true,
fee: 0,
keeper: marketOrderKeepers[BTC_USD_MARKET_ID],
data: abi.encode(marketOrderConfigurationData)
});
perpsEngine.updateSettlementConfiguration(
BTC_USD_MARKET_ID,
SettlementConfiguration.MARKET_ORDER_CONFIGURATION_ID,
newSettlementConfiguration
);
}
Also, add the following exposed function to test/harnesses/perpetuals/leaves/GlobalConfigurationHarness.sol
function workaround_getLiquidationFee() external view returns (uint128) {
GlobalConfiguration.Data storage self = GlobalConfiguration.load();
return self.liquidationFeeUsdX18;
}
Modify subsequently the script/utils/TreeProxyUtils.solto add this new function selector in the globalConfigurationHarnessSelectorarray.
- bytes4[] memory globalConfigurationHarnessSelectors = new bytes4[](11);
+ bytes4[] memory globalConfigurationHarnessSelectors = new bytes4[](12);
//...
+ globalConfigurationHarnessSelectors[11] = GlobalConfigurationHarness.workaround_getLiquidationFee.selector;
Add the following test in test/integration/perpetuals/liquidation-branch/checkLiquidatableAccounts/checkLiquidatableAccounts.t.sol
function test_DecreasingMaintenanceMargin() public {
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(BTC_USD_MARKET_ID);
changePrank({ msgSender: users.owner.account });
setFeesTo0(BTC_USD_MARKET_ID);
deal({ token: address(usdc), to: users.madara.account, give: 10_000e6 });
changePrank({ msgSender: users.madara.account });
uint128 tradingAccountId = createAccountAndDeposit(1000e6, address(usdc));
int128 sizeDelta = 1e18;
uint256 initialPrice = 10_000e18;
MockPriceFeed priceFeed = MockPriceFeed(fuzzMarketConfig.priceAdapter);
priceFeed.updateMockPrice(initialPrice);
OrderBranch(address(perpsEngine)).createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: tradingAccountId,
marketId: fuzzMarketConfig.marketId,
sizeDelta: sizeDelta
})
);
changePrank({ msgSender: marketOrderKeepers[fuzzMarketConfig.marketId] });
perpsEngine.fillMarketOrder(
tradingAccountId,
fuzzMarketConfig.marketId,
getMockedSignedReport(fuzzMarketConfig.streamId, initialPrice)
);
(UD60x18 requiredInitialMarginUsdX18, UD60x18 requiredMaintenanceMarginUsdX18, ) = perpsEngine
.exposed_getAccountMarginRequirementUsdAndUnrealizedPnlUsd(tradingAccountId, 0, SD59x18_ZERO);
uint128 liquidationFeePercentage = GlobalConfigurationHarness(address(perpsEngine)).workaround_getLiquidationFee() / 100;
uint256 initialMarginLiquidationFee = (((initialPrice * BTC_USD_IMR) / 1e18) * liquidationFeePercentage) /
(10 ** (18 + USDC_DECIMALS));
uint256 maintenanceMarginLiquidationFee = (((initialPrice * BTC_USD_MMR) / 1e18) * liquidationFeePercentage) /
(10 ** (18 + USDC_DECIMALS));
assertEq(
requiredInitialMarginUsdX18.intoUint256(),
100e18 + initialMarginLiquidationFee,
"Initial initial margin should be $100 plus liquidation fees"
);
assertEq(
requiredMaintenanceMarginUsdX18.intoUint256(),
50e18 + maintenanceMarginLiquidationFee,
"Initial maintenance margin should be $50 plus liquidation fees"
);
SD59x18 marginBalanceUsdX18 = TradingAccountHarness(address(perpsEngine)).exposed_getMarginBalanceUsd(
tradingAccountId,
SD59x18_ZERO
);
assertFalse(
TradingAccountHarness(address(perpsEngine)).exposed_isLiquidatable(
requiredInitialMarginUsdX18,
marginBalanceUsdX18
)
);
uint256 newPrice = 1000e18;
priceFeed.updateMockPrice(newPrice);
initialMarginLiquidationFee =
(((newPrice * BTC_USD_IMR) / 1e18) * liquidationFeePercentage) /
(10 ** (18 + USDC_DECIMALS));
maintenanceMarginLiquidationFee =
(((newPrice * BTC_USD_MMR) / 1e18) * liquidationFeePercentage) /
(10 ** (18 + USDC_DECIMALS));
(UD60x18 newRequiredInitialMarginUsdX18, UD60x18 newRequiredMaintenanceMarginUsdX18, ) = perpsEngine
.exposed_getAccountMarginRequirementUsdAndUnrealizedPnlUsd(tradingAccountId, 0, sd59x18(-9000));
assertEq(
newRequiredInitialMarginUsdX18.intoUint256(),
10e18 + initialMarginLiquidationFee,
"New initial margin incorrectly decreased to $10 plus liquidation fees"
);
assertEq(
newRequiredMaintenanceMarginUsdX18.intoUint256(),
5e18 + maintenanceMarginLiquidationFee,
"New maintenance margin incorrectly decreased to $5 plus liquidation fees"
);
marginBalanceUsdX18 = TradingAccountHarness(address(perpsEngine)).exposed_getMarginBalanceUsd(
tradingAccountId,
sd59x18(-9000)
);
console.log("Margin balance: %s", marginBalanceUsdX18.intoUint256() / 1e18);
assertFalse(
TradingAccountHarness(address(perpsEngine)).exposed_isLiquidatable(
newRequiredInitialMarginUsdX18,
marginBalanceUsdX18
)
);
}
Tools Used
Testing
Recommendations
Store the initial notional value of each position when it's opened and use this initial notional value for maintenance margin calculations instead of recalculating based on the current price.
In Position.sol
struct Data {
int256 size;
uint128 lastInteractionPrice;
int128 lastInteractionFundingFeePerUnit;
+ UD60x18 initialNotionalValue;
}
function update(Data storage self, Data memory newPosition) internal {
self.size = newPosition.size;
self.lastInteractionPrice = newPosition.lastInteractionPrice;
self.lastInteractionFundingFeePerUnit = newPosition.lastInteractionFundingFeePerUnit;
+ self.initialNotionalValueX18 = newPosition.initialNotionalValueX18;
}
In SettlementBranch.sol:
+ // Calculate the initial notional value of the position
+ ctx.initialNotionalValueX18 = ctx.oldPositionSizeX18.add(sizeDeltaX18).abs().intoUD60x18().mul(fillPriceX18);
// create new position in working area
ctx.newPosition = Position.Data({
size: ctx.oldPositionSizeX18.add(sizeDeltaX18).intoInt256(),
lastInteractionPrice: fillPriceX18.intoUint128(),
lastInteractionFundingFeePerUnit: ctx.fundingFeePerUnitX18.intoInt256().toInt128(),
+ initialNotionalValueX18: ctx.initialNotionalValueX18
});
Then update the calls in TradingAccountBranch::getAccountMarginBreakdownand TradingAccount::getAccountMarginRequirementUsdAndUnrealizedPnlUsdto correctly reflects the changes.
function getAccountMarginBreakdown(uint128 tradingAccountId)
external
view
returns (
SD59x18 marginBalanceUsdX18,
UD60x18 initialMarginUsdX18,
UD60x18 maintenanceMarginUsdX18,
SD59x18 availableMarginUsdX18
)
{
// ...
for (uint256 i; i < tradingAccount.activeMarketsIds.length(); i++) {
// ..
- UD60x18 notionalValueX18 = position.getNotionalValue(markPrice);
(UD60x18 positionInitialMarginUsdX18, UD60x18 positionMaintenanceMarginUsdX18) = Position
.getMarginRequirement(
- notionalValueX18
+ position.initialNotionalValueX18,
ud60x18(perpMarket.configuration.initialMarginRateX18),
ud60x18(perpMarket.configuration.maintenanceMarginRateX18)
);
// ...
}
function getAccountMarginRequirementUsdAndUnrealizedPnlUsd(
Data storage self,
uint128 targetMarketId,
SD59x18 sizeDeltaX18
)
internal
view
returns (
UD60x18 requiredInitialMarginUsdX18,
UD60x18 requiredMaintenanceMarginUsdX18,
SD59x18 accountTotalUnrealizedPnlUsdX18
)
{
if (targetMarketId != 0) {
// ...
- UD60x18 notionalValueX18 = sd59x18(position.size).add(sizeDeltaX18).abs().intoUD60x18().mul(markPrice);
// calculate margin requirements
(UD60x18 positionInitialMarginUsdX18, UD60x18 positionMaintenanceMarginUsdX18) = Position.getMarginRequirement(
- notionalValueX18,
+ position.initialNotionalValueX18,
ud60x18(perpMarket.configuration.initialMarginRateX18),
ud60x18(perpMarket.configuration.maintenanceMarginRateX18)
);
// ...
for (uint256 i; i < cachedActiveMarketsIdsLength; i++) {
//...
- UD60x18 notionalValueX18 = position.getNotionalValue(markPrice);
// calculate margin requirements
(UD60x18 positionInitialMarginUsdX18, UD60x18 positionMaintenanceMarginUsdX18) = Position.getMarginRequirement(
+ position.initialNotionalValueX18,
ud60x18(perpMarket.configuration.initialMarginRateX18),
ud60x18(perpMarket.configuration.maintenanceMarginRateX18)
);
// ...
