DeFiFoundry
60,000 USDC
View results
Submission Details
Severity: medium
Invalid

Negative margin balance handling in `TradingAccountBranch::getAccountLeverage` causes Denial of Service due to underflow

Summary

The TradingAccountBranch::getAccountLeverage function is susceptible to underflow when handling cases with disproportionately large positions relative to minimal margin balances. This can lead to incorrect leverage calculations or function reverts, potentially disrupting critical operations that rely on accurate leverage information.

Vulnerability Details

Proof of Concept

A user opens a leveraged long position significantly larger than their deposited margin. If the market experiences even a slight downturn, querying the account's leverage via TradingAccountBranch::getAccountLeverage can cause the function to fail.

Proof of Code

Copy the following test in the test suite:

function testGetAccountLeverageUnderflow() public {
// Setup
MarketConfig memory fuzzMarketConfig = getFuzzMarketConfig(DOGE_USD_MARKET_ID);
MockPriceFeed priceFeed = MockPriceFeed(fuzzMarketConfig.priceAdapter);
// Create account and deposit
deal({ token: address(usdc), to: users.naruto.account, give: 902e6 });
changePrank({ msgSender: users.naruto.account });
uint128 tradingAccountId = createAccountAndDeposit(902e6, address(usdc));
// Open a very large position
int128 positionSize = 1_000_000e18;
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: tradingAccountId,
marketId: fuzzMarketConfig.marketId,
sizeDelta: positionSize
})
);
changePrank({ msgSender: marketOrderKeepers[fuzzMarketConfig.marketId] });
perpsEngine.fillMarketOrder(
tradingAccountId,
fuzzMarketConfig.marketId,
getMockedSignedReport(fuzzMarketConfig.streamId, fuzzMarketConfig.mockUsdPrice)
);
priceFeed.updateMockPrice(fuzzMarketConfig.mockUsdPrice * 999/1000); // 0.1% decrease
// Attempt to get account leverage
vm.expectRevert();
perpsEngine.getAccountLeverage(tradingAccountId);
}

Impact

Incorrect leverage calculations can lead to mispricing of positions or misrepresentation of account health to users. It can also destabilize the system if other functions depend on accurate leverage calculations.

Although this issue does not directly cause a loss of funds, it can lead to financial losses due to incorrect decision-making based on inaccurate leverage information.

Tools Used

Manual Testing

Recommendations

Implement a check to ensure the margin balance is not negative, preventing invalid leverage calculations:

+ if (marginBalanceUsdX18.lt(UD60x18_ZERO)) {
+ return UD60x18_ZERO;
+ }

Alternatively, use the absolute value of marginBalanceUsdX18 to prevent division by a negative value, which could result in erroneous calculations:

- return totalPositionsNotionalValue.intoSD59x18().div(marginBalanceUsdX18).intoUD60x18();
+ return totalPositionsNotionalValue.intoSD59x18().div(marginBalanceUsdX18.abs()).intoUD60x18();
Updates

Lead Judging Commences

inallhonesty Lead Judge
11 months ago
inallhonesty Lead Judge 11 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement

Support

FAQs

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