Zaros Part 1

Summary

There is a maximum number of positions in which an account is entitled to open which is being set by the globalConfiguration.maxPositionsPerAccount, and validated by the validatePositionsLimit() function.
However, new position could be created in order to put the account over the maximum number of open positions.

Vulnerability Details

This vulnerability breaks a core protocol logic and invariant as it has numerous effects and dangers.
Users can now open as many accounts as they wish to have in a trading account, as opposed to being limited by the maximum number of positions every account is entitled to open.

This vulnerabilities arise as an oversight or error in the implementation check by the createMarketOrder function as opposed to the actual implementation meant by the protocol. The createMarketOrder is a function used in creating a market order for the given trading account and takes in the CreateMarketOrderParams parameter in doing so.

However, there is a check as shown below ⬇

The above check verifies if the current context of the trading account does not have a market with active positions and Validates if the trading account is under the configured positions limit, otherwise it reverts with Errors.MaxPositionsPerAccountReached.

The bug here lays in the fact that a context with no active market positions is a new market order, hence there is no way it will exceed the maximum positions limit and as such there is technically no reason to check for that, the right logic would be to check for trading account that does have a market with active positions as such accounts already have positions and are liable to reach the maximum limit.

illustration

Current implementation
Logic:

• if Alice does not have a market with active position, validate the positions limit to ensure its within the maximum position limit.

Note:

• There is technically no reason to validate if Alice positions are within the set limit, as a new market order will be the first position in such trading account.

Correct implementation
Logic:

• if Alice have a market with active position, validate the positions limit to ensure its within the maximum position limit

Note:

• This is the right time to validate the position limit, since its obvious alice already has some positions opened and should ensure Alice doesn't go beyond the limit.

Impact

The impact of this wrong logic is that users are still able to open multiple market positions even above the maximum limit being set by the protocol, as the logic is broken.

POC

Tools Used

Manual Review

Recommendations

Restructure the logic by removing the ! operator in the check