DeFiLayer 1Layer 2
14,723 OP
View results
Submission Details
Severity: high
Invalid

`verifyScrvusdByStateRoot` would break the oracle in a valid path

Summary

The ScrvusdVerifierV1.sol priceparams handling in verifyScrvusdByStateRoot() is broken cause when extracting parameters from storage proofs, if the last_profit_update or other storage slot is zero (which is a valid path), this zero value is passed as a timestamp surrogate to price calculations in the case of last_profit_update and in the case of any other slot so is it passed

This zero value however for most of the slots breaks the oracle as it flaws price calculations, profit evolution, and accounting in ScrvusdOracleV2.sol.

Vulnerability Details

NB: This report can be made for most of the slots that are read, but in this report we would focus on params[5] & params[6], i.e last_profit_update and balance_of_self

First note: ScrvusdVerifierV1::verifyScrvusdByStateRoot()

function verifyScrvusdByStateRoot(
uint256 _block_number,
bytes memory _proof_rlp
) external returns (uint256) {
bytes32 state_root = IBlockHashOracle(BLOCK_HASH_ORACLE).get_state_root(_block_number);
uint256[PARAM_CNT] memory params = _extractParametersFromProof(state_root, _proof_rlp);
// Use last_profit_update as the timestamp surrogate
return _updatePrice(params, params[5], _block_number);
}

params[5]

Going down the path of last_profit_update first since it's specifically passed in as the "timestamp surrogate" when we use the stateroot verification path unlike the headers path where we pass the block timestamp. as seen we don't check if this value is non-zero, but from examining the parameter extraction logic, we see that these parameters can be zero, per documentation:

https://github.com/CodeHawks-Contests/2025-03-curve/blob/198820f0c30d5080f75073243677ff716429dbfd/contracts/scrvusd/verifiers/ScrvusdVerifierV1.sol#L82-L112

/// @dev Extract parameters from the state proof using the given state root.
function _extractParametersFromProof(
bytes32 stateRoot,
bytes memory proofRlp
) internal view returns (uint256[PARAM_CNT] memory) {
//..snip
// Extract slot values
uint256[PARAM_CNT] memory params;
for (uint256 i = 1; i < PROOF_CNT; i++) {
Verifier.SlotValue memory slot = Verifier.extractSlotValueFromProof(
keccak256(abi.encode(PARAM_SLOTS[i])),
account.storageRoot,
proofs[i].toList()
);
|> // Slots might not exist, but typically we just read them.
params[i - 1] = slot.value;
}
return params;
}

So when extracting, we explicitly acknowledge that "slots might not exist," but the code proceeds without validating that the slot value in this case timestamp value is non-zero.

As hinted earlier on, in contrast with the other verification method:

function verifyScrvusdByBlockHash(
bytes memory _block_header_rlp,
bytes memory _proof_rlp
) external returns (uint256) {
Verifier.BlockHeader memory block_header = Verifier.parseBlockHeader(_block_header_rlp);
// ...
return _updatePrice(params, block_header.timestamp, block_header.number);
}

This method uses the actual block timestamp from the header, which is guaranteed to be non-zero for a valid block header.

Also note that in the verifier that updates the profit max unlock time we actually ensure the extracted slot value exists, see this.

Now, this issue becomes severe when examining how this timestamp value is used in ScrvusdOracleV2.vy:

def _raw_price(ts: uint256, parameters_ts: uint256) -> uint256:
parameters: PriceParams = self._obtain_price_params(parameters_ts)
return self._total_assets(parameters) * 10**18 // self._total_supply(parameters, ts)

params[6]

The same case can be made for the balance_of_self value, when extracting, all these values in the param are iterated upon in the array while being set without verification of them being non-zero and then sent directly to the update

// Extract slot values
uint256[PARAM_CNT] memory params;
for (uint256 i = 1; i < PROOF_CNT; i++) {
Verifier.SlotValue memory slot = Verifier.extractSlotValueFromProof(
keccak256(abi.encode(PARAM_SLOTS[i])),
account.storageRoot,
proofs[i].toList()
);
// Slots might not exist, but typically we just read them.
params[i - 1] = slot.value;
}
|> return params;
function verifyScrvusdByStateRoot(
uint256 _block_number,
bytes memory _proof_rlp
) external returns (uint256) {
bytes32 state_root = IBlockHashOracle(BLOCK_HASH_ORACLE).get_state_root(_block_number);
uint256[PARAM_CNT] memory params = _extractParametersFromProof(state_root, _proof_rlp);
// Use last_profit_update as the timestamp surrogate
return _updatePrice(params, params[5], _block_number);//@audit params passed here
}

Going into the oracle, if we do have the balance_of_self value as 0, the our unlocked shares calculation is effectively broken, cause we would easily return that 0 shares are unlocked, whereas all the shares should be unlocked:

https://github.com/CodeHawks-Contests/2025-03-curve/blob/198820f0c30d5080f75073243677ff716429dbfd/contracts/scrvusd/oracles/ScrvusdOracleV2.vy#L189-L214

@view
def _unlocked_shares(
full_profit_unlock_date: uint256,
profit_unlocking_rate: uint256,
last_profit_update: uint256,
balance_of_self: uint256,
ts: uint256,
) -> uint256:
unlocked_shares: uint256 = 0
if full_profit_unlock_date > ts:
# If we have not fully unlocked, we need to calculate how much has been.
unlocked_shares = profit_unlocking_rate * (ts - last_profit_update) // MAX_BPS_EXTENDED
elif full_profit_unlock_date != 0:
# All shares have been unlocked
unlocked_shares = balance_of_self #@audit
return unlocked_shares

Which would then go ahead and break the pricing logic, cause this is used as a denominator when getting the raw price, since we would have an overly inflated total_supply:

def _total_supply(p: PriceParams, ts: uint256) -> uint256:
# Need to account for the shares issued to the vault that have unlocked.
return p.total_supply - self._unlocked_shares( #audit here we have total shares as `0`
p.full_profit_unlock_date,
p.profit_unlocking_rate,
p.last_profit_update,
p.balance_of_self,
ts, # block.timestamp
)

End price is heavily deflated:

@view
def _raw_price(ts: uint256, parameters_ts: uint256) -> uint256:
"""
@notice Price replication from scrvUSD vault
"""
parameters: PriceParams = self._obtain_price_params(parameters_ts)
return self._total_assets(parameters) * 10**18 // self._total_supply(parameters, ts) #@audit since total_spply is inflated

Impact

The oracle gets broken if any of the slots we pass are inexisted and/or their value is zero.

How broken the oracle gets is now dependent on what slot was passed while being non-existent, and the report shows two of these cases, i.e when last_profit_value is 0 and also how the price would be incorrectly deflated in cases when balance_of_self is inexistent/zero, cause if the full profits has already been unlocked, i.e full_profit_unlock_date < ts, so we should have the full balance returned, but we instead have 0 returned.

Other params can also lead to broken total assets which would also affect the prices, or even broken profit evolution and gain period calculation since any of the below could then be assumed to be zero where as it isn't:

https://github.com/CodeHawks-Contests/2025-03-curve/blob/198820f0c30d5080f75073243677ff716429dbfd/contracts/scrvusd/oracles/ScrvusdOracleV2.vy#L315-L323

@external
def update_price()
#snip
self.price_params = PriceParams(
total_debt=_parameters[0],
total_idle=_parameters[1],
total_supply=_parameters[2],
full_profit_unlock_date=_parameters[3],
profit_unlocking_rate=_parameters[4],
last_profit_update=_parameters[5],
balance_of_self=_parameters[6],
)
self.price_params_ts = _ts
#snip

Tools Used

Manual review

Recommendations

All slots that are used in the oracle should be validated to be non-zero while extracting the parameters from the proof.

Alternatively if we are only looking at the window from verifyScrvusdByStateRoot() then this would be the fix

function verifyScrvusdByStateRoot(
uint256 _block_number,
bytes memory _proof_rlp
) external returns (uint256) {
bytes32 state_root = IBlockHashOracle(BLOCK_HASH_ORACLE).get_state_root(_block_number);
uint256[PARAM_CNT] memory params = _extractParametersFromProof(state_root, _proof_rlp);
// Use last_profit_update as the timestamp surrogate
+ require(params[5] > 0, "Invalid timestamp value");
return _updatePrice(params, params[5], _block_number);
}

In general though, something like is done in the verifier that updates the profit max unlock time should be implemented.

Updates

Lead Judging Commences

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

[invalid] finding-missing-proof-content-validation

- See [here]([https://github.com/CodeHawks-Contests/2025-03-curve?tab=readme-ov-file#blockhash-oracle)](https://github.com/CodeHawks-Contests/2025-03-curve?tab=readme-ov-file#blockhash-oracle) on how it is used to verify storage variable - All state roots and proofs must be verified by the OOS `StateProofVerifier` inherited as `Verifier` (where the price values and params are extracted), so there is no proof that manipulating timestamp/inputs can affect a price update - It is assumed that the OOS prover will provide accurate data and the OOS verifier will verify the prices/max unlock time to be within an appropriate bound/values - There is a account existance check in L96 of `ScrvusdVerifierV1.sol`, in which the params for price updates are extracted from

Support

FAQs

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