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

position modification is not accounted for in updateActiveMarkets

kwakudr

Summary

position modification is not accounted for when both oldPositionSize and newPositionSize are non-zero but different

Vulnerability Details

function updateActiveMarkets(
Data storage self,
uint128 marketId,
SD59x18 oldPositionSize,
SD59x18 newPositionSize
)
internal
{
GlobalConfiguration.Data storage globalConfiguration = GlobalConfiguration.load();
​
​
// if this is a new position
if (oldPositionSize.isZero() && !newPositionSize.isZero()) {
// if this account has no other active positions
if (!globalConfiguration.accountsIdsWithActivePositions.contains(self.id)) {
// then record it into global config as an account having active positions
globalConfiguration.accountsIdsWithActivePositions.add(self.id);
}
​
​
// add this market id as active for this account
self.activeMarketsIds.add(marketId);
}
// if the existing position was closed
else if (oldPositionSize.neq(SD59x18_ZERO) && newPositionSize.eq(SD59x18_ZERO)) {
// remove this market as active for this account
self.activeMarketsIds.remove(marketId);
​
​
// if the account has no more active markets
if (self.activeMarketsIds.length() == 0) {
// then remove the account from active accounts in global config
globalConfiguration.accountsIdsWithActivePositions.remove(self.id);
}
}
}

Let's say we have a trader with the following activity:

  1. Initial state: No position (oldPositionSize = 0)

  2. Opens a position: Long 10 ETH (newPositionSize = 10)

  3. Later, modifies position: Reduces to Long 5 ETH (oldPositionSize = 10, newPositionSize = 5)

Current function behavior:

  • Step 1 to 2: The function correctly adds the market to active markets.

  • Step 2 to 3: The function does nothing, because neither condition is met:

    • oldPositionSize is not zero (10 ≠ 0)

    • newPositionSize is not zero (5 ≠ 0)

Impact

The system relies on this function to track changes in position sizes, it will fail to capture these modifications. This could lead to inaccurate reporting or miscalculations in other parts of the system like liquidationbranch and settlementbranch functions

Tools Used

Manual Review

Recommendations

Add the condition for this position change.

Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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