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

Missing Access Control

sempermutans

Summary

The ScrvusdVerifierV1.sol and ScrvusdVerifierV2.sol contracts lack access control mechanisms for all price and period update functions. This vulnerability undermines the carefully implemented role-based access control system in the oracle contract (ScrvusdOracleV2.vy), allowing any external actor to call these functions and potentially manipulate critical protocol parameters.

Vulnerability Details

Severity: High

Files Affected:

  • contracts/scrvusd/verifiers/ScrvusdVerifierV1.sol

  • contracts/scrvusd/verifiers/ScrvusdVerifierV2.sol

    Functions Affected:

  • verifyScrvusdByBlockHash()

  • verifyScrvusdByStateRoot()

  • verifyPeriodByBlockHash()

  • verifyPeriodByStateRoot()

All four verification functions are defined with the external visibility modifier but lack any access control mechanisms:

// ScrvusdVerifierV1.sol
function verifyScrvusdByBlockHash(
bytes memory _block_header_rlp,
bytes memory _proof_rlp
) external returns (uint256) {
// No access control check
// ...
}
​
function verifyScrvusdByStateRoot(
uint256 _block_number,
bytes memory _proof_rlp
) external returns (uint256) {
// No access control check
// ...
}
​
// ScrvusdVerifierV2.sol
function verifyPeriodByBlockHash(
bytes memory _block_header_rlp,
bytes memory _proof_rlp
) external returns (bool) {
// No access control check
// ...
}
​
function verifyPeriodByStateRoot(
uint256 _block_number,
bytes memory _proof_rlp
) external returns (bool) {
// No access control check
// ...
}

This is particularly problematic because the oracle contract (ScrvusdOracleV2.vy) implements a proper role-based access control system:

# From ScrvusdOracleV2.vy
PRICE_PARAMETERS_VERIFIER: public(constant(bytes32)) = keccak256("PRICE_PARAMETERS_VERIFIER")
UNLOCK_TIME_VERIFIER: public(constant(bytes32)) = keccak256("UNLOCK_TIME_VERIFIER")
​
@external
def update_price(
_parameters: uint256[ALL_PARAM_CNT], _ts: uint256, _block_number: uint256
) -> uint256:
access_control._check_role(PRICE_PARAMETERS_VERIFIER, msg.sender)
# ...
​
@external
def update_profit_max_unlock_time(_profit_max_unlock_time: uint256, _block_number: uint256) -> bool:
access_control._check_role(UNLOCK_TIME_VERIFIER, msg.sender)
# ...

The oracle expects that only approved verifiers with specific roles can call its update functions. However, because the verifier contracts themselves lack access control, anyone can trigger these updates by calling the verifier functions, completely bypassing the oracle's security model.

Impact

The lack of access control on these critical functions has severe implications:

  1. Circumvention of Oracle's Security Model: The carefully designed role-based access control in the oracle is rendered ineffective since anyone can call the verifier contracts.

  2. Price Manipulation: Unauthorized actors can trigger price updates at strategically chosen times by selectively submitting proofs from specific blocks where the scrvUSD parameters would result in favorable price movements.

  3. Economic Exploitation: An attacker could monitor the chain for moments when parameters would cause advantageous price or period updates, then exploit these moments for financial gain through other protocol interactions.

  4. Timestamp Manipulation: This vulnerability compounds with the oracle's acceptance of user-provided timestamps, allowing attackers to potentially influence price calculations with manipulated timestamps.

  5. Oracle Reliability Compromise: The reliability of price data is fundamental to DeFi protocols. Unrestricted access to update functions undermines this reliability and could lead to cascading failures in dependent systems.

Tools Used

  • Manual code review

Recommendations

  1. Implement Role-Based Access Control in Verifier Contracts:

    • Integrate OpenZeppelin's AccessControl library to limit function access to authorized entities.

    • Ensure the same roles enforced in the oracle are also enforced in the verifiers.

import "@openzeppelin/contracts/access/AccessControl.sol";
​
contract ScrvusdVerifierV1 is AccessControl {
bytes32 public constant PRICE_UPDATER_ROLE = keccak256("PRICE_PARAMETERS_VERIFIER");
constructor(address _block_hash_oracle, address _scrvusd_oracle) {
BLOCK_HASH_ORACLE = _block_hash_oracle;
SCRVUSD_ORACLE = _scrvusd_oracle;
_setupRole(DEFAULT_ADMIN_ROLE, msg.sender);
}
function verifyScrvusdByBlockHash(
bytes memory _block_header_rlp,
bytes memory _proof_rlp
) external onlyRole(PRICE_UPDATER_ROLE) returns (uint256) {
// Existing implementation
}
function verifyScrvusdByStateRoot(
uint256 _block_number,
bytes memory _proof_rlp
) external onlyRole(PRICE_UPDATER_ROLE) returns (uint256) {
// Existing implementation
}
}
​
contract ScrvusdVerifierV2 is ScrvusdVerifierV1 {
bytes32 public constant PERIOD_UPDATER_ROLE = keccak256("UNLOCK_TIME_VERIFIER");
// In constructor, inherit roles from V1
function verifyPeriodByBlockHash(
bytes memory _block_header_rlp,
bytes memory _proof_rlp
) external onlyRole(PERIOD_UPDATER_ROLE) returns (bool) {
// Existing implementation
}
function verifyPeriodByStateRoot(
uint256 _block_number,
bytes memory _proof_rlp
) external onlyRole(PERIOD_UPDATER_ROLE) returns (bool) {
// Existing implementation
}
}

  1. Grant Roles to Appropriate Entities:

    • Ensure that only trusted entities (like authorized bots or governance) have the required roles.

    • Implement a proper role administration system to manage these permissions.

// In deployment script or governance function
function setupRoles(
address verifierV1,
address verifierV2,
address oracle,
address updaterBot
) external onlyAdmin {
// Grant updater role to the bot
IAccessControl(verifierV1).grantRole(PRICE_UPDATER_ROLE, updaterBot);
IAccessControl(verifierV2).grantRole(PERIOD_UPDATER_ROLE, updaterBot);
// Grant verifier roles to the verifier contracts in the oracle
IAccessControl(oracle).grantRole(PRICE_PARAMETERS_VERIFIER, verifierV1);
IAccessControl(oracle).grantRole(UNLOCK_TIME_VERIFIER, verifierV2);
}

  1. Time-Based Update Restrictions:

    • Implement cooldown periods between updates to prevent rapid manipulation.

mapping(address => uint256) public lastUpdateTime;
uint256 public constant UPDATE_COOLDOWN = 1 hours;
​
function verifyScrvusdByBlockHash(
bytes memory _block_header_rlp,
bytes memory _proof_rlp
) external onlyRole(PRICE_UPDATER_ROLE) returns (uint256) {
require(block.timestamp >= lastUpdateTime[msg.sender] + UPDATE_COOLDOWN, "Update too frequent");
lastUpdateTime[msg.sender] = block.timestamp;
// Rest of implementation
}

  1. Event Emission for Auditing:

    • Add events for all parameter updates to facilitate off-chain monitoring.

event PriceVerificationAttempted(address indexed caller, uint256 blockNumber, uint256 timestamp);
​
function verifyScrvusdByBlockHash(
bytes memory _block_header_rlp,
bytes memory _proof_rlp
) external onlyRole(PRICE_UPDATER_ROLE) returns (uint256) {
// Get the block header
Verifier.BlockHeader memory block_header = Verifier.parseBlockHeader(_block_header_rlp);
emit PriceVerificationAttempted(msg.sender, block_header.number, block_header.timestamp);
// Rest of implementation
}

By implementing these recommendations, particularly the role-based access control that aligns with the oracle's security model, the contracts would significantly improve their security posture and prevent unauthorized manipulation of critical protocol parameters.

Updates

Lead Judging Commences

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

[invalid] finding-verify-functions-lack-access-control

Invalid, all state roots and proofs must be verified by the OOS `StateProofVerifier` inherited as `Verifier`, so there is no proof that a permisionless `verify`functions allow updating malicious prices

Support

FAQs

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