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

Incorrect validation in `isIncreasing()`.

bladesec

Github link

https://github.com/Cyfrin/2024-07-zaros/blob/d687fe96bb7ace8652778797052a38763fbcbb1b/src/perpetuals/leaves/Position.sol#L171

Summary

isIncreasing() returns a wrong flag for some cases.

Vulnerability Details

isIncreasing() checks if the position size is increasing.

function isIncreasing(
uint128 tradingAccountId,
uint128 marketId,
int128 sizeDelta
)
internal
view
returns (bool)
{
Data storage self = Position.load(tradingAccountId, marketId);
​
// If position is being opened (size is 0) or if position is being increased (sizeDelta direction is same as
// position size)
// For a positive position, sizeDelta should be positive to increase, and for a negative position, sizeDelta
// should be negative to increase.
// This also implicitly covers the case where if it's a new position (size is 0), it's considered an increase.
return self.size == 0 || (self.size > 0 && sizeDelta > 0) || (self.size < 0 && sizeDelta < 0); //@audit incorrect validation
}

If the position is increased, createMarketOrder() checks if the market and settlement are enabled and reverts if not.

But isIncreasing() returns false when the absolute value of position size is increased from negative to positive(from -10 to 100), or from positive to negative(from 10 to -100).

So traders can increase their positions by reversing the direction twice for disabled markets(from -10 to 100, and 100 to -1000).

Impact

Traders can open positions(or increase positions) on disabled markets.

Tools Used

Manual Review

Recommendations

isIncreasing() should return true when the absolute size of the position is increased.

Updates

Lead Judging Commences

inallhonesty Lead Judge about 1 year ago
Submission Judgement Published
Validated
Assigned finding tags:

Disable market limitation can be bypassed

Support

FAQs

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