Summary
in fillOrder()
Use tradingAccount.validateMarginRequirement(totalFees=orderFee+ settlementFee) to check for sufficient collateral
But when calculating the collateral value, it is multiplied by marginCollateralConfiguration.loanToValue
So the actual collateral value deducted will be smaller than totalFees=orderFee+ settlementFee
Vulnerability Details
in fillOrder()
We need to check if the collateral is sufficient
function _fillOrder(
uint128 tradingAccountId,
uint128 marketId,
uint128 settlementConfigurationId,
SD59x18 sizeDeltaX18,
UD60x18 fillPriceX18
)
...
(
UD60x18 requiredInitialMarginUsdX18,
UD60x18 requiredMaintenanceMarginUsdX18,
SD59x18 accountTotalUnrealizedPnlUsdX18
) = tradingAccount.getAccountMarginRequirementUsdAndUnrealizedPnlUsd(marketId, sizeDeltaX18);
ctx.shouldUseMaintenanceMargin = !ctx.isIncreasing && !ctx.oldPositionSizeX18.isZero();
ctx.requiredMarginUsdX18 =
ctx.shouldUseMaintenanceMargin ? requiredMaintenanceMarginUsdX18 : requiredInitialMarginUsdX18;
tradingAccount.validateMarginRequirement(
ctx.requiredMarginUsdX18,
tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18),
@> ctx.orderFeeUsdX18.add(ctx.settlementFeeUsdX18)
);
}
@> tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: globalConfiguration.orderFeeRecipient,
settlementFeeRecipient: globalConfiguration.settlementFeeRecipient
}),
pnlUsdX18: ctx.pnlUsdX18.lt(SD59x18_ZERO) ? ctx.pnlUsdX18.abs().intoUD60x18() : UD60x18_ZERO,
orderFeeUsdX18: ctx.orderFeeUsdX18,
settlementFeeUsdX18: ctx.settlementFeeUsdX18
});
First: tradingAccount.validateMarginRequirement() directly subtracts orderFee+ settlementFee
And finally: using tradingAccount.deductAccountMargin(orderFee+ settlementFee)
But when we value the collateral getMarginBalanceUsd(), we multiply by marginCollateralConfiguration.loanToValue
adjustedBalanceUsdX18 = marginCollateralConfiguration.getPrice().mul(ud60x18(balance)).mul(ud60x18(marginCollateralConfiguration. loanToValue)
So tradingAccount.deductAccountMargin(orderFee+ settlementFee) actually reduces the valuation by = (orderFee+ settlementFee) * loanToValue
Impact
Due to the existence of marginCollateralConfiguration.loanToValue, using validateMarginRequirement() to determine if there is enough collateral is inaccurate and may result in failure to execute the order properly
Tools Used
Recommendations
Check for sufficient collateral at the end of the fillOrder() method.
function _fillOrder(
uint128 tradingAccountId,
uint128 marketId,
uint128 settlementConfigurationId,
SD59x18 sizeDeltaX18,
UD60x18 fillPriceX18
)
....
ctx.shouldUseMaintenanceMargin = !ctx.isIncreasing && !ctx.oldPositionSizeX18.isZero();
ctx.requiredMarginUsdX18 =
ctx.shouldUseMaintenanceMargin ? requiredMaintenanceMarginUsdX18 : requiredInitialMarginUsdX18;
// reverts if the trader can't satisfy the appropriate margin requirement
- tradingAccount.validateMarginRequirement(
- ctx.requiredMarginUsdX18,
- tradingAccount.getMarginBalanceUsd(accountTotalUnrealizedPnlUsdX18),
- ctx.orderFeeUsdX18.add(ctx.settlementFeeUsdX18)
- );
}
...
tradingAccount.deductAccountMargin({
feeRecipients: FeeRecipients.Data({
marginCollateralRecipient: globalConfiguration.marginCollateralRecipient,
orderFeeRecipient: globalConfiguration.orderFeeRecipient,
settlementFeeRecipient: globalConfiguration.settlementFeeRecipient
}),
pnlUsdX18: ctx.pnlUsdX18.lt(SD59x18_ZERO) ? ctx.pnlUsdX18.abs().intoUD60x18() : UD60x18_ZERO,
orderFeeUsdX18: ctx.orderFeeUsdX18,
settlementFeeUsdX18: ctx.settlementFeeUsdX18
});
+ tradingAccount.validateMarginRequirement(
+ ctx.requiredMarginUsdX18,
+ tradingAccount.getMarginBalanceUsd(SD59x18_ZERO), // will load the latest collateral value
+ SD59x18_ZERO
+ );
emit LogFillOrder(
msg.sender,
tradingAccountId,
marketId,
sizeDeltaX18.intoInt256(),
fillPriceX18.intoUint256(),
ctx.orderFeeUsdX18.intoUint256(),
ctx.settlementFeeUsdX18.intoUint256(),
ctx.pnlUsdX18.intoInt256(),
ctx.fundingFeePerUnitX18.intoInt256()
);
}
