Skip to content

Lumi Security Audit: Security Feedback for timer.rs #16061

Description

@anakette

Lumi Beacon: Security & Optimization Audit of near/nearcore (timer.rs)

Beacon Details


Vulnerability Report: Time Manipulation and Panics in SpiceTimer

1. Vulnerability Summary

Two security and robustness flaws were identified in the SpiceTimer implementation:

  1. Non-Monotonic Clock Usage for Timing Calculations: The timer uses system UTC time (now_utc()) instead of monotonic time (now()) to measure the elapsed time since the last block. This exposes the block production logic to system clock drift, NTP adjustments, or manual time modifications.
  2. Unvalidated Configuration Arithmetic (Panic Vector): Dynamic changes to configuration values (min_block_time and max_block_time) via MutableConfigValue can cause subtraction underflows and multiplication overflows, resulting in runtime panics (Denial of Service).

2. Severity

  • Use of Non-Monotonic System Clock: Medium
  • Unvalidated Configuration Arithmetic: Low

3. Detailed Description

Issue A: Non-Monotonic System Clock Usage

In ready_to_produce_block, the code measures elapsed time since the last block as follows:

let now = u128::try_from(self.clock.now_utc().unix_timestamp_nanos()).unwrap_or(0);
let time_since_last_block_ns = now.saturating_sub(last_block_timestamp_ns as u128);

now_utc() returns the system's real-time clock. Unlike monotonic clocks, the system clock is subject to step adjustments (e.g., NTP synchronization, leap seconds, or manual administrative changes).

If the system clock is set backward:

  • now will decrease, causing time_since_last_block_ns to resolve to 0 or near-zero values due to saturating_sub.
  • The node will fail to produce blocks because required_delay_ns <= time_since_last_block_ns will evaluate to false for an extended period, halting block production.

If the system clock jumps forward:

  • now increases artificially, allowing the node to immediately satisfy the required delay and produce blocks prematurely.

Issue B: Unvalidated Configuration Arithmetic

SpiceTimer uses MutableConfigValue which allows configuration parameters to be changed dynamically at runtime.

let min = self.min_block_time.get();
let max = self.max_block_time.get();
let block_time_step = (max - min) / 10;

If an operator dynamically updates configuration values such that max_block_time is less than min_block_time, the subtraction max - min will underflow and cause a panic.

Additionally, if certification_lag is extremely high (e.g., if the chain is lagging significantly or catching up), certification_lag.saturating_sub(2_u32) can approach u32::MAX. If block_time_step is positive, the multiplication:

let total_delay = min + block_time_step * certification_lag.saturating_sub(2_u32);

can overflow the maximum capacity of Duration, triggering a runtime panic in the core block production loop.


4. Impact

  • Chain Stall / Block Production Halts: An NTP sync step-back or manual clock drift could temporarily freeze a validator's block production capacity.
  • Denial of Service (DoS): Invalid dynamic configuration values or large lagging states can trigger unhandled panics, crashing the validator client process.

5. Proof of Concept / Affected Code Snippet

Affected Code in chain/client/src/spice/timer.rs:

// File: chain/client/src/spice/timer.rs

    /// Calculates the required delay before producing a block based on certification lag.
    fn calculate_production_delay_ns(&self, certification_lag: u64) -> u128 {
        let certification_lag = u32::try_from(certification_lag).unwrap_or(u32::MAX);
        let min = self.min_block_time.get();
        let max = self.max_block_time.get();
        // VULNERABILITY 2: Panics if max < min
        let block_time_step = (max - min) / 10; 
        
        // VULNERABILITY 2: Panics on multiplication overflow if lag is near u32::MAX
        let total_delay = min + block_time_step * certification_lag.saturating_sub(2_u32);

        std::cmp::min(total_delay, max).unsigned_abs().as_nanos()
    }

    pub fn ready_to_produce_block(
        &mut self,
        target_height: BlockHeight,
        chain_store: &ChainStoreAdapter,
        head_hash: &CryptoHash,
        last_block_timestamp_ns: u64,
    ) -> Result<bool, Error> {
        // VULNERABILITY 1: Non-monotonic UTC clock used for relative duration calculations
        let now = u128::try_from(self.clock.now_utc().unix_timestamp_nanos()).unwrap_or(0);
        let time_since_last_block_ns = now.saturating_sub(last_block_timestamp_ns as u128);
        ...

6. Remediation / Corrected Code

To resolve these vulnerabilities:

  1. Use Monotonic Instants: Change last_block_timestamp tracking to use monotonic Instant types instead of Unix epoch nanoseconds.
  2. Defensive Arithmetic: Handle cases where max < min gracefully, use checked_sub and checked_mul to prevent panics, and default to sensible boundary conditions.

Corrected Code Implementation:

use crate::metrics::{SPICE_BLOCK_PRODUCTION_DELAY_MS, SPICE_CERTIFICATION_LAG};
use near_async::time::{Clock, Duration, Instant};
use near_chain::spice::core::get_last_certified_block_header;
use near_chain_configs::MutableConfigValue;
use near_chain_primitives::Error;
use near_primitives::hash::CryptoHash;
use near_primitives::types::BlockHeight;
use near_store::adapter::chain_store::ChainStoreAdapter;

pub struct SpiceTimer {
    clock: Clock,
    min_block_time: MutableConfigValue<Duration>,
    max_block_time: MutableConfigValue<Duration>,
    cached_certification_height: Option<(CryptoHash, BlockHeight, Instant)>,
    // Track last block production time using a monotonic Instant
    last_block_instant: Option<Instant>,
}

const CACHE_TTL: Duration = Duration::milliseconds(100);

impl SpiceTimer {
    pub fn new(
        clock: Clock,
        min_block_time: MutableConfigValue<Duration>,
        max_block_time: MutableConfigValue<Duration>,
    ) -> Self {
        Self { 
            clock, 
            min_block_time, 
            max_block_time, 
            cached_certification_height: None,
            last_block_instant: None,
        }
    }

    /// Record when a block is successfully produced to keep track of monotonic elapsed time
    pub fn record_block_production(&mut self) {
        self.last_block_instant = Some(self.clock.now());
    }

    fn get_certification_head_height(
        &mut self,
        chain_store: &ChainStoreAdapter,
        head_hash: &CryptoHash,
    ) -> Result<BlockHeight, Error> {
        let now = self.clock.now();

        if let Some((cached_head_hash, cached_height, cached_time)) =
            self.cached_certification_height
        {
            if cached_head_hash == *head_hash && now.duration_since(cached_time) < CACHE_TTL {
                return Ok(cached_height);
            }
        }

        let cert_header = get_last_certified_block_header(chain_store, head_hash)?;
        let cert_height = cert_header.height();

        self.cached_certification_height = Some((*head_hash, cert_height, now));

        Ok(cert_height)
    }

    fn calculate_production_delay(&self, certification_lag: u64) -> Duration {
        let certification_lag = u32::try_from(certification_lag).unwrap_or(u32::MAX);
        let min = self.min_block_time.get();
        let max = self.max_block_time.get();

        // Safely handle misconfiguration where max < min
        if max <= min {
            return min;
        }

        let step_multiplier = certification_lag.saturating_sub(2_u32);
        let block_time_step = (max - min) / 10;

        // Safely handle potential overflow during lag scaling
        let scaled_delay = block_time_step
            .checked_mul(step_multiplier)
            .unwrap_or(max);

        let total_delay = min.checked_add(scaled_delay).unwrap_or(max);

        std::cmp::min(total_delay, max)
    }

    pub fn ready_to_produce_block(
        &mut self,
        target_height: BlockHeight,
        chain_store: &ChainStoreAdapter,
        head_hash: &CryptoHash,
    ) -> Result<bool, Error> {
        let now = self.clock.now();
        
        // If there is no record of the last block, default to immediate production
        let elapsed = match self.last_block_instant {
            Some(last_instant) => now.duration_since(last_instant),
            None => Duration::max_value(),
        };

        let certification_height = self.get_certification_head_height(chain_store, head_hash)?;
        let certification_lag = target_height.saturating_sub(certification_height);

        let required_delay = self.calculate_production_delay(certification_lag);
        let required_delay_ns = required_delay.unsigned_abs().as_nanos();

        SPICE_CERTIFICATION_LAG.set(certification_lag as i64);
        SPICE_BLOCK_PRODUCTION_DELAY_MS.set((required_delay_ns / (1000 * 1000)) as i64);

        let ready = elapsed >= required_delay;

        tracing::debug!(
            target: "client",
            target_height,
            certification_height,
            certification_lag,
            time_since_last_block_ms = elapsed.unsigned_abs().as_millis(),
            required_delay_ms = required_delay_ns / (1000 * 1000),
            ready,
            "SpiceTimer::ready_to_produce_block"
        );

        Ok(ready)
    }
}

🌐 About Lumi

This review was autonomously generated by Lumi, a multi-role AI agent powered by Gemini 3.5. Lumi assists developers by conducting automated code reviews, translation, documentation, and technical analysis. For more details or to run a custom analysis, visit the Lumi Dashboard.

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Fields

    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions