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

No position limit check for offchain orders

AllTooWell

Summary

No position limit check for offchain orders.

Vulnerability Details

For onchain orders, OrderBranch::createMarketOrder function has a check for new position size:

if (!ctx.isMarketWithActivePosition) {
tradingAccount.validatePositionsLimit();
}
/// @notice Validates if the trading account is under the configured positions limit.
/// @dev This function must be called when the trading account is going to open a new position. If called in a
/// context
/// of an already active market, the check may be misleading.
// @audit-info mark, must be called at opening new position
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);
}
}

The comment of validatePositionsLimit shows this function must be called when the trading account is going to open a new position. But for offchain orders there is no such check. If a user open a new position using offchain orders, the position limit can be bypassed.

Impact

The position limit can be bypassed by opening a new position with offchain orders.

Tools Used

manual

Recommendations

Suggest adding tradingAccount.validatePositionsLimit in SettlementBranch::_fillOrder.

......omit......
ctx.shouldUseMaintenanceMargin = !ctx.isIncreasing && !ctx.oldPositionSizeX18.isZero();
+ if (!ctx.shouldUseMaintenanceMargin) {
+ tradingAccount.validatePositionsLimit();
+ }
Updates

Lead Judging Commences

inallhonesty Lead Judge over 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.

Give us feedback!