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

Maximum open position per account can be bypassed via filling offchain orders.

blutorque

Summary

Vulnerability Details

When a user createMarketOrder(), the tradingAccount.validatePositionsLimit() check ensures that the user does not exceed the maxPositionsPerAccount limit. If the limit is exceeded, it reverts the order creation transaction.

if (activePositionsLength >= maxPositionsPerAccount) {
revert Errors.MaxPositionsPerAccountReached(self.id, activePositionsLength, maxPositionsPerAccount);
}

However, the issue is that offchain orders do not have such restrictions, which allows traders to create as many positions as they want under their tradingAccountId. A malicious user could sign different OffchainOrder.Data under different marketId instead of calling OrderBranch.createMarketOrder(), allowing them to bypass the position limit check on their trades.

library OffchainOrder {
/// @notice {OffchainOrder} namespace storage structure.
/// @param tradingAccountId The trading account id that created the order offchain.
/// @param marketId The target market id of the offchain order.
/// @param sizeDelta The size delta of the offchain order.
/// @param targetPrice The minimum or maximum price of the offchain order.
/// @param shouldIncreaseNonce Whether the trading account's nonce should be incremented or not.
/// @param nonce The signed index used to verify whether a given order is still valid or not.
/// @param salt A random 32 bytes value generated and signed offchain to distinguish an offchain order.
/// @param v ECDSA signature recovery id.
/// @param r ECDSA signature output.
/// @param s ECDSA signature output.
struct Data {
uint128 tradingAccountId;
uint128 marketId;
int128 sizeDelta;
uint128 targetPrice;
bool shouldIncreaseNonce;
uint120 nonce;
bytes32 salt;
uint8 v;
bytes32 r;
bytes32 s;
}
}

Impact

An account can have limitless open position under different markets

Tools Used

Manual review

Recommendations

Add following snippet to thefillOffchainOrder() function of SettlementBranch.sol,

if (!tradingAccount.isMarketWithActivePosition(ctx.offchainOrder.marketId)) {
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!