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

Hardcoded Storage Slots

sempermutans

Summary

Both the ScrvusdVerifierV1.sol and ScrvusdVerifierV2.sol contracts use hardcoded addresses and storage slot numbers which could become invalid if the target scrvUSD contract is upgraded or changes its storage layout. This creates a brittle dependency that could break the verifiers and disrupt the price oracle system.

Vulnerability Details

Severity: Medium

Files Affected:

  • contracts/scrvusd/verifiers/ScrvusdVerifierV1.sol

  • contracts/scrvusd/verifiers/ScrvusdVerifierV2.sol

Code Affected:

  • Hardcoded address and storage slots in ScrvusdVerifierV1:

// ScrvusdVerifierV1.sol - Lines 23-32
address constant SCRVUSD = 0x0655977FEb2f289A4aB78af67BAB0d17aAb84367;
bytes32 constant SCRVUSD_HASH = keccak256(abi.encodePacked(SCRVUSD));
​
// Storage slots of parameters
uint256[PROOF_CNT] internal PARAM_SLOTS = [
uint256(0), // filler for account proof
uint256(21), // total_debt
uint256(22), // total_idle
uint256(20), // totalSupply
uint256(38), // full_profit_unlock_date
uint256(39), // profit_unlocking_rate
uint256(40), // last_profit_update
uint256(keccak256(abi.encode(18, SCRVUSD))) // balanceOf(self)
];

  • Hardcoded storage slot in ScrvusdVerifierV2:

// ScrvusdVerifierV2.sol - Line 14
uint256 internal PERIOD_SLOT = 37; // profit_max_unlock_time

These hardcoded values create rigid dependencies on the current implementation and storage layout of the scrvUSD contract. If the scrvUSD contract is upgraded and its storage layout changes, the verifier contracts would extract incorrect data from the new slots, leading to incorrect price or period updates.

The importance of these slot values becomes even clearer when examining the oracle contract (ScrvusdOracleV2.vy), which relies on these extracted parameters for critical price calculations:

# From ScrvusdOracleV2.vy
@external
def update_price(
_parameters: uint256[ALL_PARAM_CNT], _ts: uint256, _block_number: uint256
) -> uint256:
# ...
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],
)
# ...

The oracle uses these parameters in complex calculations to determine the price:

@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)

If the verifiers extract incorrect data due to storage slot changes, this would directly affect these price calculations, potentially causing significant financial impact.

Impact

The use of hardcoded storage slots has several implications:

  1. Fragility During Upgrades: If the scrvUSD contract is upgraded and changes its storage layout, the verifiers would extract incorrect data, potentially causing protocol-wide issues.

  2. Incorrect Parameter Extraction: Changes in storage layout could lead to extracting unrelated data from the wrong slots, causing incorrect price or period updates in the oracle.

  3. Protocol Disruption: In a DeFi system, incorrect price data could lead to significant financial losses through mispriced assets, incorrect liquidations, or exploitable arbitrage opportunities.

  4. Maintenance Challenges: Each time the target contract is upgraded, all verifier contracts would need to be redeployed with updated slot numbers, increasing operational complexity.

  5. Limited Adaptability: The contracts cannot adapt to any changes in the protocol without redeployment, creating potential downtime during upgrades.

  6. Complex Price Calculation Impact: As seen in the oracle's _raw_price function, the extracted parameters are used in division operations, so incorrect values could lead to division by zero errors or wildly inaccurate prices.

Tools Used

  • Manual code review

Recommendations

  1. Implement Configurable Storage Slots:

    • Replace hardcoded slots with configurable parameters that can be updated by authorized administrators.

contract ScrvusdVerifierV1 is AccessControl {
address public SCRVUSD;
bytes32 public SCRVUSD_HASH;
mapping(uint256 => uint256) public PARAM_SLOTS;
constructor(address _block_hash_oracle, address _scrvusd_oracle, address _scrvusd) {
BLOCK_HASH_ORACLE = _block_hash_oracle;
SCRVUSD_ORACLE = _scrvusd_oracle;
_setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
setScrvusdAddress(_scrvusd);
initializeDefaultSlots();
}
function setScrvusdAddress(address _scrvusd) public onlyRole(DEFAULT_ADMIN_ROLE) {
SCRVUSD = _scrvusd;
SCRVUSD_HASH = keccak256(abi.encodePacked(_scrvusd));
}
function setParamSlot(uint256 index, uint256 slot) public onlyRole(DEFAULT_ADMIN_ROLE) {
require(index < PROOF_CNT, "Index out of bounds");
PARAM_SLOTS[index] = slot;
emit ParamSlotUpdated(index, slot);
}
function initializeDefaultSlots() internal {
PARAM_SLOTS[0] = 0; // filler for account proof
PARAM_SLOTS[1] = 21; // total_debt
PARAM_SLOTS[2] = 22; // total_idle
// Initialize other slots...
}
event ParamSlotUpdated(uint256 index, uint256 slot);
}
​
contract ScrvusdVerifierV2 is ScrvusdVerifierV1 {
uint256 public PERIOD_SLOT;
constructor(address _block_hash_oracle, address _scrvusd_oracle, address _scrvusd)
ScrvusdVerifierV1(_block_hash_oracle, _scrvusd_oracle, _scrvusd) {
PERIOD_SLOT = 37; // Default value
}
function setPeriodSlot(uint256 _slot) public onlyRole(DEFAULT_ADMIN_ROLE) {
PERIOD_SLOT = _slot;
emit PeriodSlotUpdated(_slot);
}
event PeriodSlotUpdated(uint256 slot);
}

  1. Consider Using EIP-1967 Storage Slots:

    • For unstructured storage proxy patterns, consider using standardized storage slots as defined in EIP-1967, which are designed to avoid collisions during upgrades.

  2. Implement Slot Verification Mechanism:

    • Add functionality to verify that slots contain expected data types or value ranges before using them for critical operations.

function _verifySlotCompatibility(uint256 slotValue, uint256 slotIndex) internal pure returns (bool) {
// Implement verification logic based on expected data for each slot
if (slotIndex == 1) { // total_debt
return slotValue < 1e30; // Reasonable upper bound check
} else if (slotIndex == 2) { // total_idle
return slotValue < 1e30; // Reasonable upper bound check
} else if (slotIndex == 3) { // totalSupply
return slotValue > 0; // Must be positive
}
// Add checks for other parameters
return true;
}
​
function _extractParametersFromProof(...) internal view returns (uint256[PARAM_CNT] memory) {
// Existing implementation
for (uint256 i = 1; i < PROOF_CNT; i++) {
// Extract slot value
params[i - 1] = slot.value;
// Verify compatibility based on slot index
require(_verifySlotCompatibility(params[i - 1], i), "Incompatible slot value");
}
return params;
}

  1. Versioned Contracts with Upgrade Coordination:

    • Implement a versioning system for verifier contracts to track compatibility with scrvUSD contract versions.

    • Coordinate upgrades between the scrvUSD contract, oracle, and verifier contracts.

contract ScrvusdVerifierV1 is AccessControl {
string public constant COMPATIBLE_SCRVUSD_VERSION = "1.0.0";
// Add function to check version compatibility
function checkVersionCompatibility(string calldata scrvusdVersion) public pure returns (bool) {
return keccak256(abi.encodePacked(COMPATIBLE_SCRVUSD_VERSION)) ==
keccak256(abi.encodePacked(scrvusdVersion));
}
// Rest of implementation
}

  1. External Storage Layout Registry:

    • Consider implementing a registry contract that maps contract versions to their storage layouts, allowing verifiers to lookup the correct slots dynamically.

By implementing these recommendations, particularly the configurable storage slots, the contracts would be more adaptable to changes in the target contract's storage layout, reducing the risk of disruption during upgrades and improving overall protocol resilience.

Updates

Lead Judging Commences

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

[invalid] finding-upgradeable-verifier-contracts

Invalid, - srCRVUSD is a minimal proxy, meaning it can never by upgraded, see [here](https://www.cyfrin.io/blog/upgradeable-proxy-smart-contract-pattern#:~:text=Minimal%20proxies%20are%20distinct%20from,provide%20upgrade%20or%20authorization%20functionality.) and [here](https://www.rareskills.io/post/eip-1167-minimal-proxy-standard-with-initialization-clone-pattern) for more info. - Even if srcrvUSD is migrated in the future via a new minimal proxy contract deployment (which is highly unlikely), the verifier contracts can be migrated along with it via revoking the access-control within the `ScrvusdOracleV2.vy` and then granting access to a new oracle. This is also not within the scope of this contest.

Support

FAQs

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