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

The prices are changed suddenly when `self.profit_max_unlock_time` is changed.

danielarmstrong

Summary

The prices in ScrvusdOracleV2 are hardly dependes on self.profit_max_unlock_time.
So if self.profit_max_unlock_time is changed by verifier, the prices are changed suddenly.

Vulnerability Details

The ScrvusdOracleV2.vy#_obtain_price_params() function which is used for calculating prices is as follows.

@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
245 period: uint256 = self.profit_max_unlock_time
if params.last_profit_update + period >= parameters_ts:
return params
​
249 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):
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

As we can see above, period (=self.profit_max_unlock_time) is critical parameter to simulate price params.

But ScrvusdOracleV2.vy#update_profit_max_unlock_time() function updates this critical parameter suddenly.

@external
def update_profit_max_unlock_time(_profit_max_unlock_time: uint256, _block_number: uint256) -> bool:
"""
@notice Update price using `_parameters`
@param _profit_max_unlock_time New `profit_max_unlock_time` value
@param _block_number Block number of parameters to linearize updates
@return Boolean whether value changed
"""
access_control._check_role(UNLOCK_TIME_VERIFIER, msg.sender)
# Allowing same block updates for fixing bad blockhash provided (if possible)
assert self.last_block_number <= _block_number, "Outdated"
self.last_block_number = _block_number
​
prev_value: uint256 = self.profit_max_unlock_time
self.profit_max_unlock_time = _profit_max_unlock_time
return prev_value != _profit_max_unlock_time

This is serious error.

Impact

The prices are changed suddenly when verifier changes self.profit_max_unlock_time.
In this case, _smoothed_price does not interact to prevent sudden change.

Tools Used

Manual Review

Recommendations

Modify ScrvusdOracleV2.vy#update_profit_max_unlock_time() function as follows.

def update_profit_max_unlock_time(_profit_max_unlock_time: uint256, _block_number: uint256) -> bool:
"""
@notice Update price using `_parameters`
@param _profit_max_unlock_time New `profit_max_unlock_time` value
@param _block_number Block number of parameters to linearize updates
@return Boolean whether value changed
"""
access_control._check_role(UNLOCK_TIME_VERIFIER, msg.sender)
# Allowing same block updates for fixing bad blockhash provided (if possible)
assert self.last_block_number <= _block_number, "Outdated"
self.last_block_number = _block_number
​
++ self.last_prices = [self._price_v0(), self._price_v1(), self._price_v2()]
++ self.last_update = block.timestamp
​
prev_value: uint256 = self.profit_max_unlock_time
self.profit_max_unlock_time = _profit_max_unlock_time
return prev_value != _profit_max_unlock_time
Updates

Lead Judging Commences

0xnevi Lead Judge 5 months ago
Submission Judgement Published
Invalidated
Reason: Lack of quality
Assigned finding tags:

[invalid] finding-change-in-max-update-time-influence

This issue and its duplicates lack sufficient proof of the impact of a sudden change in `profit_max_unlock_time`. Both price parameters and `profit_max_unlock_time` can be adjusted immediately, However, the whole purpose of `_smoothed_price` is to limit sudden updates. This is performed when the raw price and last price is compared within the `_price_v0/v1/v2` function calls to limit price updates to `max_change` The slowed price lag can then be safely arbitrage as mentioned in the docs > Smoothing is introduced for sudden updates, so the price slowly catches up with the price, while the pool is being arbitraged safely. Though, smoothing limits the upper bound of price growth. Therefore, we consider that scrvUSD will never be over 60% APR.

Support

FAQs

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