c4-bot-5 commented 7 months ago

Lines of code

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1570

Vulnerability details

Accurate collateral calculations are crucial for maintaining the solvency of the Panoptic protocol. Underestimations can lead to insufficient collateral, risking the protocol's stability during market downturns, while overestimations can tie up unnecessary capital, reducing liquidity and trading efficiency.

The collateral requirement for spreads between two option legs is calculated as follows:

Loc

https://github.com/code-423n4/2024-04-panoptic/blob/main/contracts/CollateralTracker.sol#L1570

spreadRequirement = (notional < notionalP)
    ? Math.unsafeDivRoundingUp((notionalP - notional) * contracts, notional)
    : Math.unsafeDivRoundingUp((notional - notionalP) * contracts, notionalP);

This calculation can cause significant rounding errors due to integer division, particularly when the difference between notional and notionalP is minimal compared to their absolute values.

audit details

However, by far the most important functionality of the CollateralTracker is to calculate the collateral requirement for every account and position. Each time positions are minted or burned in Panoptic, the CollateralTracker updates the collateral balances and provides information on the collateral requirement, ensuring that the protocol remains solvent and we retain the ability to liquidate distressed positions when needed.

Impact

The precision in collateral calculations impacts not just individual trades but the overall health and functionality of the trading platform which makes impact in this case significant.

Traders with limited capital may find themselves unable to participate in certain trades, or may incur higher opportunity costs as their funds are tied up unnecessarily.
Traders might perceive positions as riskier than they are, or the platform may inadvertently encourage over-collateralization, affecting the natural balance and hedging strategies in place. This could lead to misaligned risk perceptions and decisions.
Inaccuracies in collateral calculations might expose the platform to compliance risks if they're interpreted as failing to provide accurate accountings or as misleading users about the terms of engagement.

Proof of Concept

Suppose we have:

notional (amount moved in one leg of the spread): 1,000,000 tokens
notionalP (amount moved in the other leg of the spread): 1,000,001 tokens
contracts (size of the position): 10,000 contracts

This setup reflects a common scenario in financial derivatives where two legs are close in value but not identical, often seen in calendar spreads or tight vertical spreads.

Calculation Without Scaling: Using the _computeSpread function, where notional < notionalP:

Difference: notionalP - notional = 1,000,001 - 1,000,000 = 1
Numerator before division: 1 * 10,000 = 10,000

The division to be performed is:

spreadRequirement = Math.unsafeDivRoundingUp(10,000, 1,000,000) = 1

What this Result Implies:

The actual decimal precise arithmetic would be:
```
10,000 / 1,000,000 = 0.01
```
But, since Solidity cannot handle decimal points, the division of 10,000 by 1,000,000 in integer arithmetic results in 0. The unsafeDivRoundingUp function then rounds this up to 1.
The expected collateral requirement should ideally be close to 0.01 but instead, the rounding results in an overestimation to 1, which is significantly larger than the actual requirement by a factor of 100 times.

This demonstrates a scenario where small differences relative to the amounts involved lead to rounding that considerably overestimates the needed collateral. Such a rounding error could impact trading strategies, leading to inefficient capital usage where traders must lock up more capital than is theoretically necessary due to a computational artifact.

Poc Code

def calculate_spread_requirement(notional, notionalP, contracts):
    # Calculate the difference and the product
    difference = abs(notionalP - notional)
    product = difference * contracts

    # Integer division to simulate Solidity's behavior
    if notional < notionalP:
        spread_requirement = -(-product // notional)  # Equivalent to Math.unsafeDivRoundingUp in Solidity
    else:
        spread_requirement = -(-product // notionalP)  # Using Python's way to round up in integer division

    return spread_requirement

def calculate_spread_requirement_float(notional, notionalP, contracts):
    # Calculate the difference and the product
    difference = abs(notionalP - notional)
    product = difference * contracts

    # Floating-point division for accurate calculation
    if notional < notionalP:
        spread_requirement = product / notional
    else:
        spread_requirement = product / notionalP

    return spread_requirement

# Scenario parameters
notional = 1000000
notionalP = 1000001
contracts = 10000

# Calculate requirements
integer_result = calculate_spread_requirement(notional, notionalP, contracts)
float_result = calculate_spread_requirement_float(notional, notionalP, contracts)

# Display results
print(f"Integer division result (Solidity-like): {integer_result}")
print(f"Floating-point division result (Accurate): {float_result}")

Output

Integer division result (Solidity-like): 1
Floating-point division result (Accurate): 0.01

Tools Used

Manual

Recommended Mitigation Steps

Scale up the calculations before the division and scale them back down afterward.

uint256 scaleFactor = 1e18;
uint256 difference = (notional < notionalP) ? (notionalP - notional) : (notional - notionalP);
uint256 scaledNumerator = difference * contracts * scaleFactor;
spreadRequirement = (notional < notionalP)
    ? Math.unsafeDivRoundingUp(scaledNumerator, notional)
    : Math.unsafeDivRoundingUp(scaledNumerator, notionalP);

// Scale down the final spreadRequirement by the scaleFactor to get back to the correct unit:
spreadRequirement = spreadRequirement / scaleFactor;

Assessed type

Math

dyedm1 commented 7 months ago

Not sure I see the problem with this? The collateral requirement has to be a certain quantity of tokens, so we either allow users to have a 0 collateral req (risky) or round conservatively in favor of the protocol to the higher req.

Picodes commented 7 months ago

I agree with the sponsor here that the magnitude of the error isn't significant and that there is no issue is slightly overestimating collateral req.

c4-judge commented 7 months ago

Picodes marked the issue as unsatisfactory: Overinflated severity

code-423n4 / 2024-04-panoptic-findings

Overestimation of collateral requirements due to rounding errors in _computeSpread function #545