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

Arithmetic underflow in PerpMarket::getMarkPrice causes a DoS with large short positions

0x5chn0uf

Summary

The PerpMarket::getMarkPrice function is susceptible to arithmetic underflow issues when processing large negative skewDelta values that are bigger than the market skewScale. This leads to incorrect negative price calculations, making the market order creation to fail.

Vulnerability Details

function getMarkPrice(
Data storage self,
SD59x18 skewDelta,
UD60x18 indexPriceX18
)
internal
view
returns (UD60x18 markPrice)
{
SD59x18 skewScale = sd59x18(self.configuration.skewScale.toInt256());
SD59x18 skew = sd59x18(self.skew);
​
SD59x18 priceImpactBeforeDelta = skew.div(skewScale);
@> SD59x18 newSkew = skew.add(skewDelta); // @ audit - What if skewDelta is negative and abs(skewDelta) > newSkew? Answer: newSkew < 0
@> SD59x18 priceImpactAfterDelta = newSkew.div(skewScale); // @audit - what if abs(newSkew) > skewScale? Answer: > 1
// ^ @audit - So priceImpactAfterDelta < -1
​
SD59x18 cachedIndexPriceX18 = indexPriceX18.intoSD59x18();
​
UD60x18 priceBeforeDelta =
cachedIndexPriceX18.add(cachedIndexPriceX18.mul(priceImpactBeforeDelta)).intoUD60x18();
@> UD60x18 priceAfterDelta =
cachedIndexPriceX18.add(cachedIndexPriceX18.mul(priceImpactAfterDelta)).intoUD60x18();
// ^ @audit: Underflow as abs(cachedIndexPriceX18.mul(priceImpactAfterDelta)) is > cachedIndexPriceX18
// because abs(priceImpactAfterDelta) > 1 and priceAfterDelta is unsigned
markPrice = priceBeforeDelta.add(priceAfterDelta).div(ud60x18Convert(2));
}

Proof of Concept

  1. User opens a large short position which respects all the conditions of OI, skew, and position size.

  2. The protocol panics when attempting to compute the filling price mark price based on the mark price.

Proof of Code

Add this test to test/integration/perpetuals/trading-account-branch/getAccountMarginBalances/getAccountMarginBalances.t.sol:

function testExtremeShortPosition() public {
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(BTC_USD_MARKET_ID);
​
// Set up the initial state
deal({ token: address(usdc), to: users.naruto.account, give: 1_000_000_000e6 });
changePrank({ msgSender: users.naruto.account });
​
uint128 tradingAccountId = createAccountAndDeposit(1_000_000_000e6, address(usdc));
int128 maxAllowedSize = int128(uint128((fuzzMarketConfig.skewScale + 1e18)));
int128 largeSizeDelta = -maxAllowedSize;
​
// Open the large short position
vm.expectRevert(); // Expect a revert here
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({ tradingAccountId: tradingAccountId, marketId: fuzzMarketConfig.marketId, sizeDelta: largeSizeDelta })
);
}

Alternatively, you can set this fuzz test in test/integration/perpetuals/perp-market-branch/getMarkPrice/getMarkPrice.t.sol:

function testFuzz_getMarkPrice(uint256 initialPrice, uint256 positionSize) public {
// We want the position size to be higher than the asset skew scale (for demo, min = 1% higher)
positionSize = bound(positionSize, BTC_USD_SKEW_SCALE * 101/100, uint256(type(int256).max));
// For demonstration purpose, we need to price to have trailing 0 because of roundings
// 1 * -1.01 = -1, thus invalidating the test
initialPrice = bound(initialPrice, 1e2, positionSize);
​
// PRBMath_SD59x18_IntoUint256_Underflow(positionSize)
vm.expectRevert();
perpsEngine.getMarkPrice(BTC_USD_MARKET_ID, initialPrice, -int256(positionSize));
}

Impact

The arithmetic underflow issue in PerpMarket::getMarkPrice has severe implications for the protocol's functionality and security:

  • Large short positions cause the creation of market orders to fail, effectively preventing legitimate trading activities and potentially locking users out of managing their positions.

  • System Instability: The vulnerability in a core function like getMarkPrice compromises the reliability of the entire perpetual trading system. This instability could lead to cascading failures in other dependent protocol functions.

Tools Used

Fuzz Testing & Manual testing

Recommendations

Implement input validation to ensure that all input parameters are within expected ranges. Ideally, you want to limit the size of an position to the maximum skew scale of the market. It could be done in PerpMarket::checkTradeSize:

function checkTradeSize(Data storage self, SD59x18 sizeDeltaX18) internal view {
+ UD60x18 sizeDelta = sizeDeltaX18.abs().intoUD60x18()
+ if (sizeDelta.lt(ud60x18(self.configuration.minTradeSizeX18))) {
- if (sizeDeltaX18.abs().intoUD60x18().lt(ud60x18(self.configuration.minTradeSizeX18))) {
revert Errors.TradeSizeTooSmall();
}
+ if (sizeDelta.gt(ud60x18(self.configuration.skewScale))) {
+ revert Errors.TradeSizeTooBig();
+ }
}
Updates

Lead Judging Commences

inallhonesty Lead Judge
about 1 year ago
inallhonesty Lead Judge 12 months ago
Submission Judgement Published
Invalidated
Reason: Non-acceptable severity

Support

FAQs

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