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

Lack of validation for `marketOrderMinLifetime` in `configureSystemParameters` will lead to orders cancelling without any delay

Panthers

Relevant Link

Summary

The configureSystemParameters function in the GlobalConfigurationBranch contract does not validate the marketOrderMinLifetime parameter. This oversight can lead to a scenario where newly created orders can be cancel directly without any delay.

Vulnerability Details

The configureSystemParameters function sets various system parameters and does validation for all params except marketOrderMinLifetime. If marketOrderMinLifetime is not initialzed or set to zero, it will cause the MarketOrder:checkPendingOrder function to always return true if order is not pending, leading to orders being canceled with any delay. Which is not what is expected, according to comments in the OrderBranch:cancelMarketOrder,

reverts if a trader has a pending order and that pending order hasn't existed for the minimum order lifetime; pending orders can't be cancelled until they have existed for the minimum order lifetime

See the following code:

​
function configureSystemParameters(
uint128 maxPositionsPerAccount,
uint128 marketOrderMinLifetime,
uint128 liquidationFeeUsdX18,
address marginCollateralRecipient,
address orderFeeRecipient,
address settlementFeeRecipient,
address liquidationFeeRecipient,
uint256 maxVerificationDelay
)
external
onlyOwner
{
if (maxPositionsPerAccount == 0) {
revert Errors.ZeroInput("maxPositionsPerAccount");
}
​
if (liquidationFeeUsdX18 == 0) {
revert Errors.ZeroInput("liquidationFeeUsdX18");
}
​
if (marginCollateralRecipient == address(0)) {
revert Errors.ZeroInput("marginCollateralRecipient");
}
​
if (orderFeeRecipient == address(0)) {
revert Errors.ZeroInput("orderFeeRecipient");
}
​
if (settlementFeeRecipient == address(0)) {
revert Errors.ZeroInput("settlementFeeRecipient");
}
​
if (liquidationFeeRecipient == address(0)) {
revert Errors.ZeroInput("liquidationFeeRecipient");
}
​
if (maxVerificationDelay == 0) {
revert Errors.ZeroInput("maxVerificationDelay");
}
​
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
​
globalConfiguration.maxPositionsPerAccount = maxPositionsPerAccount;
globalConfiguration.marketOrderMinLifetime = marketOrderMinLifetime;
globalConfiguration.liquidationFeeUsdX18 = liquidationFeeUsdX18;
globalConfiguration.marginCollateralRecipient = marginCollateralRecipient;
globalConfiguration.orderFeeRecipient = orderFeeRecipient;
globalConfiguration.settlementFeeRecipient = settlementFeeRecipient;
globalConfiguration.liquidationFeeRecipient = liquidationFeeRecipient;
globalConfiguration.maxVerificationDelay = maxVerificationDelay;
​
emit LogConfigureSystemParameters(
msg.sender, maxPositionsPerAccount, marketOrderMinLifetime, liquidationFeeUsdX18
);
}

The OrderBranch:cancelMarketOrder function relies on MarketOrder:checkPendingOrder to ensure that a market order can only be canceled after it has existed for at least marketOrderMinLifetime seconds.

function cancelMarketOrder(uint128 tradingAccountId) external {
// load existing trading account; reverts for non-existent account
// enforces `msg.sender == owner` so only account owner can cancel
// pending orders
TradingAccount.loadExistingAccountAndVerifySender(tradingAccountId);
​
// load trader's pending order; reverts if no pending order
MarketOrder.Data storage marketOrder = MarketOrder.loadExisting(tradingAccountId);
​
// reverts if a trader has a pending order and that pending order hasn't
// existed for the minimum order lifetime; pending orders can't be cancelled
// until they have existed for the minimum order lifetime
marketOrder.checkPendingOrder();
​
// reset pending order details
marketOrder.clear();
​
emit LogCancelMarketOrder(msg.sender, tradingAccountId);
}

The MarketOrder.checkPendingOrder function checks if the current timestamp minus the order's timestamp is less than marketOrderMinLifetime. So it will always allow the cancellation of market orders, regardless of how recently they were created. This could have significant impacts on the trading system's behavior and security.

function checkPendingOrder(Data storage self) internal view {
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
uint128 marketOrderMinLifetime = globalConfiguration.marketOrderMinLifetime;
​
if (
self.timestamp != 0 && marketOrderMinLifetime != 0
&& block.timestamp - self.timestamp <= marketOrderMinLifetime
) {
revert Errors.MarketOrderStillPending(self.timestamp);
}
}
​

Impact

Market orders can be canceled immediately after they are created, without any enforced waiting period. Traders might use this to rapidly place and cancel orders, potentially manipulating the market or testing trading strategies without committing to any trade for a meaningful period. Traders could exploit the ability to place and immediately cancel orders to gather information about market movements or to influence the market without significant risk. Traders could place large orders to temporarily affect prices and then cancel them immediately, causing price fluctuations that might benefit their other positions.

Tools Used

Manual Review

Recommendations

Ensure that marketOrderMinLifetime is set to a reasonable minimum value. For example, setting a minimum threshold can prevent immediate cancellations and reduce the risk of manipulation and instability.

Updates

Lead Judging Commences

inallhonesty Lead Judge
over 1 year ago
inallhonesty Lead Judge over 1 year ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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

Give us feedback!