DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: low
Valid

`maxPositionsPerAccount` may be exceeded

Summary

When creating market orders, the functions ensures that if a new position is being created in a new market, a revert happens if this new position would put the account over the maximum number of open positions.
However, for offchain orders, this check will not be performed.

Vulnerability Details

Offchain orders are signed by a user and provided to the offchain orders keeper who is responsible for filling them. They allow users to specify a targetPrice and whether the trading account's nonce should be incremented. The nonce is an index used to verify if a given order is still valid, and the salt is a random 32 bytes value used to distinguish an offchain order.

Each offchain order is also signed with ECDSA so that the order keeper can verify the signer is the account owner. The offchain order keeper fills the order if the targetPrice condition is met and the order is valid. Once an offchain order has been filled it is marked as filled to prevent it from being replayed.

However, this whole process in fillOffchainOrders() does not check if maxPositionsPerAccount is being respected in the event that a new position is being created in a new market.

Notice how this is handled in createMarketOrder():

// find if account has active position in this market
ctx.isMarketWithActivePosition = tradingAccount.isMarketWithActivePosition(params.marketId);
if (!ctx.isMarketWithActivePosition) {
>> tradingAccount.validatePositionsLimit();
}

In other words, a new position cannot be created as long as this new position would put the account over the maximum number of open positions.

However for offchain orders, this check will not be made since these orders are just signed offchain and submitted.

Impact

A user can manage to bypass the limit (maxPositionsPerAccount) by submitting offchain orders instead of using createMarketOrder()

Tools Used

Manual Review

Recommendations

The maxPositionsPerAccount should be checked for offchain orders as well during filling.

Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Validated
Assigned finding tags:

`maxPositionsPerAccount` may be exceeded by combining onchain and offchain orders

Support

FAQs

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