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

Missing Position Existence Check in `getAccountMarginBreakdown` Function

Summary

The getAccountMarginBreakdown function in the TradingAccountBranch contract iterates over the activeMarketsIds set to calculate margin requirements and unrealized P&L for each market. However, it does not verify if a position actually exists for each marketId in the set. If a position is not found, the function may revert with an error or produce incorrect results.

Vulnerability Details

The function assumes that every marketId in the activeMarketsIds set corresponds to an existing position in the Position library. However, this may not always be the case. If a position has been closed or liquidated, it may still be present in the activeMarketsIds set, but the corresponding Position.Data struct may have been cleared or deleted.

for (uint256 i; i < tradingAccount.activeMarketsIds.length(); i++) {
uint128 marketId = tradingAccount.activeMarketsIds.at(i).toUint128();
PerpMarket.Data storage perpMarket = PerpMarket.load(marketId);
Position.Data storage position = Position.load(tradingAccountId, marketId); // Potential error if position doesn't exist
// ... (rest of the loop)
}

If the Position.load function is called with a marketId for which no position exists, it may revert with an error (if the Position library has a check for existence) or return an empty or invalid Position.Data struct, leading to incorrect calculations in the getAccountMarginBreakdown function.

Impact

If the function fails to account for non-existent positions, the calculated margin breakdown (margin balance, initial margin, maintenance margin, available margin) may be incorrect, leading to inaccurate risk assessments and potentially incorrect liquidation decisions.

Tools Used

Manual review

Recommendations

Before accessing the Position.Data struct for a given marketId, add a check to verify if the position exists. This can be done by checking if the Position.Data struct has a non-zero value for a specific field (e.g., size) or by adding a helper function to the Position library to check for existence.

for (uint256 i; i < tradingAccount.activeMarketsIds.length(); i++) {
uint128 marketId = tradingAccount.activeMarketsIds.at(i).toUint128();
Position.Data storage position = Position.load(tradingAccountId, marketId);
if (position.size != 0) { // Check if position exists
// ... (rest of the loop)
}
}
Updates

Lead Judging Commences

inallhonesty Lead Judge 10 months ago
Submission Judgement Published
Invalidated
Reason: Too generic
Assigned finding tags:

Missing Position Existence Check in `getAccountMarginBreakdown` Function

If a position has been closed or liquidated, it may still be present in the `activeMarketsIds` set, but the corresponding `Position.Data` struct may have been cleared or deleted.

Support

FAQs

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