Storage Proofs
Curve
DeFiLayer 1Layer 2
14,723 OP
View results
Previous Next
Submission Details
Severity: medium
Invalid

Unbounded Loop in _obtain_price_params Function

sempermutans

Summary

The ScrvusdOracleV2.vy contract contains a potentially problematic loop in the _obtain_price_params function. While the loop does include a bound parameter, it can still iterate up to MAX_V2_DURATION times (defined as 4 * 12 * 4 = 192), which could lead to excessive gas consumption or even transaction failures due to hitting block gas limits in certain scenarios.

Vulnerability Details

Severity: Medium

Files Affected:

  • contracts/scrvusd/oracles/ScrvusdOracleV2.vy

Functions Affected:

  • _obtain_price_params()

The problematic loop is found in the _obtain_price_params function:

@view
def _obtain_price_params(parameters_ts: uint256) -> PriceParams:
"""
@notice Obtain Price parameters true or assumed to be true at `parameters_ts`.
Assumes constant gain(in crvUSD rewards) through distribution periods.
@param parameters_ts Timestamp to obtain parameters for
@return Assumed `PriceParams`
"""
params: PriceParams = self.price_params
period: uint256 = self.profit_max_unlock_time
if params.last_profit_update + period >= parameters_ts:
return params
​
number_of_periods: uint256 = min(
(parameters_ts - params.last_profit_update) // period,
self.max_v2_duration,
)
​
# locked shares at moment params.last_profit_update
gain: uint256 = (
params.balance_of_self * (params.total_idle + params.total_debt) // params.total_supply
)
params.total_idle += gain * number_of_periods
​
# functions are reduced from `VaultV3._process_report()` given assumptions with constant gain
for _: uint256 in range(number_of_periods, bound=MAX_V2_DURATION): # <-- Potentially large loop
new_balance_of_self: uint256 = (
params.balance_of_self
* (params.total_supply - params.balance_of_self) // params.total_supply
)
params.total_supply -= (
params.balance_of_self * params.balance_of_self // params.total_supply
)
params.balance_of_self = new_balance_of_self
​
if params.full_profit_unlock_date > params.last_profit_update:
# copy from `VaultV3._process_report()`
params.profit_unlocking_rate = params.balance_of_self * MAX_BPS_EXTENDED // (
params.full_profit_unlock_date - params.last_profit_update
)
else:
params.profit_unlocking_rate = 0
​
params.full_profit_unlock_date += number_of_periods * period
params.last_profit_update += number_of_periods * period
​
return params

Several concerning aspects of this code:

  1. The loop iteration count is determined by number_of_periods, which is bounded by self.max_v2_duration. This value is configurable but has a default of 4 * 6 = 24 and can go up to MAX_V2_DURATION = 4 * 12 * 4 = 192.

  2. The constant MAX_V2_DURATION (192) is used as the loop bound parameter, meaning the function is protected against infinite loops, but could still iterate many times.

  3. Each iteration performs multiple divisions and multiplications, which are relatively gas-intensive operations.

  4. This function is called by _raw_price(), which is used in multiple public functions including price queries and price updates.

Impact

The unbounded loop vulnerability could lead to several issues:

  1. Gas Limit Exceedance: If number_of_periods is large, the loop could consume enough gas to exceed block gas limits, causing transactions to fail. This is particularly concerning for price updates which are critical for protocol operation.

  2. Denial of Service: A prolonged period without updates could lead to a large number_of_periods value, potentially making price updates impossible due to gas limitations, effectively creating a denial of service condition.

  3. Inconsistent Query Results: View functions might execute locally without gas constraints, but transactions that need to use the same calculations on-chain might fail, leading to inconsistencies between what users see and what they can execute.

  4. Gas Cost Unpredictability: The gas cost of interacting with the contract becomes highly variable and unpredictable, depending on how much time has passed since the last update.

  5. Financial Impact: If price updates become impossible due to gas limitations, it could lead to stale prices being used for financial decisions, potentially causing losses for users.

While the risk is mitigated somewhat by the configurable max_v2_duration parameter and the hard cap of MAX_V2_DURATION, it remains a significant concern for the contract's gas efficiency and reliability.

Tools Used

  • Manual code review

Recommendations

  1. Implement Batch Processing:

    • Modify the function to process a fixed number of periods at a time and track progress:

# Add state variables to track processing progress
last_processed_update: public(uint256)
processing_in_progress: public(bool)
​
@external
def process_periods_batch(_batch_size: uint256 = 10) -> bool:
"""Process a batch of periods and return whether more processing is needed"""
assert not self.processing_in_progress, "Processing already in progress"
params: PriceParams = self.price_params
period: uint256 = self.profit_max_unlock_time
# If processing was started before
start_update: uint256 = self.last_processed_update if self.last_processed_update > 0 else params.last_profit_update
if start_update + period >= block.timestamp:
# No processing needed
return False
self.processing_in_progress = True
remaining_periods: uint256 = min(
(block.timestamp - start_update) // period,
self.max_v2_duration,
)
# Process only a limited batch
periods_to_process: uint256 = min(remaining_periods, _batch_size)
# Process the batch (simplified)
# ...process logic here...
# Update the tracking
self.last_processed_update = start_update + periods_to_process * period
self.processing_in_progress = False
# Return whether more processing is needed
return remaining_periods > periods_to_process

  1. Optimize Loop Operations:

    • Simplify the calculations inside the loop to reduce gas consumption:

for _: uint256 in range(number_of_periods, bound=MAX_V2_DURATION):
# Pre-calculate common expressions
supply_minus_balance: uint256 = params.total_supply - params.balance_of_self
balance_squared: uint256 = params.balance_of_self * params.balance_of_self
# Perform calculations with fewer divisions
new_balance_of_self: uint256 = params.balance_of_self * supply_minus_balance // params.total_supply
params.total_supply -= balance_squared // params.total_supply
params.balance_of_self = new_balance_of_self

  1. Implement a Maximum Time Gap Limit:

    • Add a check to prevent processing extremely large gaps:

@view
def _obtain_price_params(parameters_ts: uint256) -> PriceParams:
params: PriceParams = self.price_params
period: uint256 = self.profit_max_unlock_time
# Limit the maximum time gap that can be processed
max_allowed_gap: uint256 = 30 * 86400 # 30 days
effective_ts: uint256 = max(parameters_ts, params.last_profit_update + max_allowed_gap)
if params.last_profit_update + period >= effective_ts:
return params
number_of_periods: uint256 = min(
(effective_ts - params.last_profit_update) // period,
self.max_v2_duration,
)
# Rest of implementation

  1. Use Logarithmic Approximation for Large Gaps:

    • For very large time gaps, switch to a more efficient mathematical approximation:

@view
def _obtain_price_params(parameters_ts: uint256) -> PriceParams:
params: PriceParams = self.price_params
period: uint256 = self.profit_max_unlock_time
time_gap: uint256 = parameters_ts - params.last_profit_update
if time_gap <= period:
return params
number_of_periods: uint256 = min(time_gap // period, self.max_v2_duration)
# For large gaps, use a more efficient calculation
if number_of_periods > 20: # Threshold for switching to efficient method
# Efficient approximation for many periods
# This would use mathematical formulas to approximate the result
# without iterating through each period
return self._approximate_params_for_large_gap(params, number_of_periods)
# Regular processing for smaller gaps
# ...existing implementation...

  1. Lower Default and Maximum Values:

    • Reduce the default and maximum values for max_v2_duration to ensure the loop doesn't consume too much gas:

# In the constructor
self.max_v2_duration = 4 * 3 # 3 months instead of 6

  1. Add Gas Monitoring:

    • Add monitoring for high gas consumption scenarios to alert administrators:

@view
def _obtain_price_params(parameters_ts: uint256) -> PriceParams:
# Existing implementation
# Add an estimated gas usage log if periods are high
if number_of_periods > 10:
log HighGasUsageWarning(parameters_ts, number_of_periods)
# Rest of implementation

By implementing these recommendations, particularly the batch processing approach or mathematical approximations for large time gaps, the contract would be more resilient against gas-related issues and provide more consistent behaviour regardless of the time between updates.

Updates

Lead Judging Commences

0xnevi Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Incorrect statement
Assigned finding tags:

[invalid] finding-obtain-price-unbounded-loop

Invalid, In the verifier contracts, each price param count is restricted to 7 as per `PARAM_CNT`

Support

FAQs

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