Summary
getAccountLeverage() may revert due to underflow error.
Vulnerability Details
getAccountLeverage() returns the current leverage of a given account id, based on its cross margin collateral and open positions.
This function first gets the trading account's marginBalanceUsdX18, data type SD59x18:
SD59x18 marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(tradingAccount.getAccountUnrealizedPnlUsd());
Then iterates the trading account's active markets to get totalPositionsNotionalValue, data type is UD60x18:
function getAccountLeverage(uint128 tradingAccountId) external view returns (UD60x18) {
UD60x18 totalPositionsNotionalValue;
...
for (uint256 i; i < tradingAccount.activeMarketsIds.length(); i++) {
...
totalPositionsNotionalValue = totalPositionsNotionalValue.add(positionNotionalValueX18);
}
}
At last, totalPositionsNotionalValue is converted to type SD59x18, and is divided by marginBalanceUsdX18, the result value is converted to UD60x18 and returned:
return totalPositionsNotionalValue.intoSD59x18().div(marginBalanceUsdX18).intoUD60x18();
The problem is that marginBalanceUsdX18 can be negative if the trading account is in a loss (negative unPnl), so the result value is negative, the transaction will be reverted when it tries to convert the negative SD59x18 value to UD60x18.
Please run the PoC in fillMarketOrder.t.sol to verify:
function testAudit() public {
address alice = makeAddr("Alice");
deal(address(usdc), alice, 2000e6);
vm.startPrank(alice);
usdc.approve(address(perpsEngine), 2000e6);
uint128 tradingAccountId = createAccountAndDeposit(2000e6, address(usdc));
perpsEngine.createMarketOrder(
OrderBranch.CreateMarketOrderParams({
tradingAccountId: tradingAccountId,
marketId: BTC_USD_MARKET_ID,
sizeDelta: 1e18
})
);
vm.stopPrank();
vm.startPrank(marketOrderKeepers[BTC_USD_MARKET_ID]);
bytes memory mockSignedReport = getMockedSignedReport(BTC_USD_STREAM_ID, MOCK_BTC_USD_PRICE);
perpsEngine.fillMarketOrder(tradingAccountId, BTC_USD_MARKET_ID, mockSignedReport);
vm.stopPrank();
updateMockPriceFeed(BTC_USD_MARKET_ID, MOCK_BTC_USD_PRICE * 9 / 10);
(
SD59x18 marginBalanceUsdX18,
,
,
) = perpsEngine.getAccountMarginBreakdown(tradingAccountId);
assertTrue(marginBalanceUsdX18.unwrap() < 0);
vm.expectRevert();
UD60x18 leverage = perpsEngine.getAccountLeverage(tradingAccountId);
}
Impact
getAccountLeverage() reverts due to underflow error.
Tools Used
Manual Review
Recommendations
Return ud60x18(type(uint256).max) if marginBalanceUsdX18 is a negative value.
function getAccountLeverage(uint128 tradingAccountId) external view returns (UD60x18) {
TradingAccount.Data storage tradingAccount = TradingAccount.loadExisting(tradingAccountId);
SD59x18 marginBalanceUsdX18 = tradingAccount.getMarginBalanceUsd(tradingAccount.getAccountUnrealizedPnlUsd());
UD60x18 totalPositionsNotionalValue;
if (marginBalanceUsdX18.isZero()) return marginBalanceUsdX18.intoUD60x18();
+ if (marginBalanceUsdX18.lt(SD59x18_ZERO)) return ud60x18(type(uint256).max);
...
}
