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

MAX_V2_DURATION and the default max_v2_duration are incorrectly calculated and will lead to incorrect price calculations when used

vanshikag

Details

ScrvusdOracle.vy creates a constant MAX_V2_DURATION and a variable max_v2_duration that are set to the wrong values. While the set_max_v2_duration() function can change the value of max_v2_duration, it cannot set it to within 16 units of the intended MAX_V2_DURATION. This error happens in the following lines of code:

MAX_V2_DURATION: constant(uint256) = 4 * 12 * 4 # 4 years
.
.
.
self.max_v2_duration = 4 * 6 # half a year

4 years is 52 * 4 weeks, and the assumption that every month has 4 weeks causes MAX_V2_DURATION to be 16 weeks short of the intended value. Likewise, the default value of max_v2_duration is 2 weeks short of half a year. max_v2_duration is used to cap gains in total_idle funds in _obtain_price_params(). It is also used to cap the number of iterations of the following loop in _obtain_price_params(), interfering with balance_of_self calculations:

# number_of_periods can take a maximum value of `max_v2_duration`; the explicit 'bound' is redundant.
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

_obtain_price_params() therefore returns incorrect values whenever the number_of_periods calculated by it is within the error range of max_v2_duration. This function is called by _raw_price and is therefore called every time any price needs to be calculated. Functions relying on values modified incorrectly by _obtain_price_params() will also return the wrong values. For example, _total_assets() relies on total_idle funds which is modified by _obtain_price_params() depending on min(weeks_since_last_update, max_v2_duration).

Impact

Incorrectly set max_v2_duration returns incorrect prices and parameters when price update is happening within the error range of max_v2_duration. The impact is a failure of the protocol, but the likelihood is low. I am therefore submitting this as a medium vulnerability.

Proof Of Concept

Add the following test to tests/scrvusd/oracle/unitary/test_v2.py and run with pytest -k test_max_v2_duration.

def test_max_v2_duration(soracle, verifier, admin):
ts = boa.env.evm.patch.timestamp
week = 7 * 86400
# dummy values
params = [5, 6, 360, ts*10, 5, ts, 120]
​
with boa.env.prank(verifier):
soracle.update_price(params, ts, 5)
a = soracle.raw_price(ts, ts) # 32727272727272727867
​
with boa.env.prank(admin):
with boa.reverts():
# value of max_v2_duration cannot be set to 4 years
soracle.set_max_v2_duration(4 * 7 * 52)
# setting it to half a year, which is the intended default
# comment out this line to check default (incorrect) behavior
soracle.set_max_v2_duration(26)
boa.env.time_travel(30 * week)
ts = boa.env.evm.patch.timestamp
with boa.env.prank(verifier):
soracle.update_price(params, ts, 5)
​
b = soracle.raw_price(ts, ts) # 3168674698795180726 when max_v2_duration = 24; 2955056179775280899 when max_v2_duration = 26
assert a == b # gives a & b values

Tools Used

pytest v8.3.4, manual code review

Recommendation

Fix the values of MAX_V2_DURATION and max_v2_duration to reflect the intended maximum timespans.

- MAX_V2_DURATION: constant(uint256) = 4 * 12 * 4 # 4 years
+ MAX_V2_DURATION: constant(uint256) = 52 * 4 # 4 years
.
.
.
- self.max_v2_duration = 4 * 6 # half a year
+ self.max_v2_duration = 26 # half a year
Updates

Lead Judging Commences

0xnevi Lead Judge 7 months ago
Submission Judgement Published
Invalidated
Reason: Design choice
Assigned finding tags:

[invalid] finding-MAX_V2_DURATION

This is simply an approximation. I don't believe there is any incorrect logic here, given as long as this duration of growth is consistently applied, there will arguably be no incorrect oracle prices here. Additionally, I highly doubt there will be a instance where 48 weeks has passed since the last update.

Support

FAQs

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