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

`fillOffchainOrders()` doesn't validate the `maxPositionsPerAccount` requirement.

Github link

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/branches/SettlementBranch.sol#L186

Summary

fillOffchainOrders() doesn't validate the maxPositionsPerAccount requirement.

Vulnerability Details

While creating a market order in createMarketOrder(), it calls validatePositionsLimit().

function validatePositionsLimit(Data storage self) internal view {
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
uint256 maxPositionsPerAccount = globalConfiguration.maxPositionsPerAccount;
uint256 activePositionsLength = self.activeMarketsIds.length();
if (activePositionsLength >= maxPositionsPerAccount) {
revert Errors.MaxPositionsPerAccountReached(self.id, activePositionsLength, maxPositionsPerAccount);
}
}

And validatePositionsLimit() reverts if the trading account has more positions than maxPositionsPerAccount.

But for offchain orders, there is no such a requirement while creating an offchain order(via offchain) and filling it.

Impact

A trading account would have more positions than maxPositionsPerAccount after filling offchain orders.

Tools Used

Manual Review

Recommendations

fillOffchainOrders() should validate the maxPositionsPerAccount requirement.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year 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.