Skip to content

Lumi Security Audit: Security Feedback for mod.rs #16091

Description

@anakette

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

Beacon Details


GitHub Issue Report: Configuration Panic and LRU Cache Pollution in PeerStore

1. Vulnerability Summary

Two distinct issues have been identified in the PeerStore component of nearcore:

  1. Uncaught Startup Panic (Denial of Service): During the initialization of the PeerStore, the configuration parameter peer_states_cache_size is converted into a NonZeroUsize and immediately unwrapped. If a node operator configures this cache size to 0, the node will crash immediately upon startup with an unhandled panic.
  2. LRU Cache Pollution / Runtime Inefficiency: During the periodic maintenance of the peer store, the unbanning logic uses get_mut to update a peer's status. In an LRU (Least Recently Used) cache, get_mut promotes the accessed key to the head of the cache (Most Recently Used). This incorrectly elevates inactive, formerly banned peers, causing active or useful historical peers to be prematurely evicted from the cache.

2. Severity

  • Configuration Panic / DoS: Medium (Leads to immediate process termination/Denial of Service on misconfiguration).
  • LRU Cache Pollution: Low (Performance degradation and cache inefficiency).

3. Detailed Description

Issue A: Panic on Zero-Value Cache Size

In PeerStore::new, the cache is instantiated using the lru::LruCache crate:

let mut peer_id_2_state =
    LruCache::new(NonZeroUsize::new(config.peer_states_cache_size as usize).unwrap());

NonZeroUsize::new returns None if the passed argument is 0. Calling .unwrap() on a None variant triggers a thread panic. If a node deployment configuration file contains peer_states_cache_size: 0, the peer manager fails to initialize, preventing the node from starting.

Issue B: LRU Cache Pollution during Unban

The periodic maintenance routine Inner::update calls self.unban(now) to release peers whose ban window has expired:

fn unban(&mut self, now: time::Utc) {
    ...
    for peer_id in &to_unban {
        if let Err(err) = self.peer_unban(&peer_id) { ... }
    }
}

This delegates to Inner::peer_unban, which uses get_mut to fetch the peer state and modify its status:

fn peer_unban(&mut self, peer_id: &PeerId) -> anyhow::Result<()> {
    if let Some(peer_state) = self.peer_states.get_mut(peer_id) {
        peer_state.status = KnownPeerStatus::NotConnected;
    } else { ... }
}

In standard LRU cache implementations (such as the lru crate), get_mut updates the key's position to make it the most recently used item. Consequently, previously banned (and currently inactive) peers are placed at the top of the cache. If a node experiences high churn or bans a large number of malicious/offline endpoints, the periodic unbanning process will continuously shift these dead addresses to the MRU position, displacing valid, healthy offline peers and reducing peer discovery efficiency.


4. Impact

  • Impact of Issue A: Immediate crash of the neard process during initialization. In automated environments, this can trigger infinite restart loops.
  • Impact of Issue B: Degraded peer selection. Since the peer store has a hard capacity limit, healthy inactive nodes are evicted in favor of recently unbanned nodes that may no longer be online or functional.

5. Proof of Concept / Affected Code Snippet

Affected Code: chain/network/src/peer_manager/peer_store/mod.rs

  • Unvalidated Unwrapping (Line 233-234):
let mut peer_id_2_state =
    LruCache::new(NonZeroUsize::new(config.peer_states_cache_size as usize).unwrap());
  • LRU Promotion via get_mut (Line 131-137):
fn peer_unban(&mut self, peer_id: &PeerId) -> anyhow::Result<()> {
    if let Some(peer_state) = self.peer_states.get_mut(peer_id) {
        peer_state.status = KnownPeerStatus::NotConnected;
    } else {
        bail!("Peer {} is missing in the peer store", peer_id);
    }
    Ok(())
}

6. Remediation / Corrected Code

To resolve these issues:

  1. Validate config.peer_states_cache_size and return a clean error if it is configured to 0, rather than crashing.
  2. Replace get_mut with peek_mut in peer_unban to modify the peer status in-place without altering its LRU eviction priority.

Corrected Code Implementation

impl Inner {
    // cspell:words unban unbans
    fn peer_unban(&mut self, peer_id: &PeerId) -> anyhow::Result<()> {
        // Use peek_mut instead of get_mut to prevent LRU cache pollution
        if let Some(peer_state) = self.peer_states.peek_mut(peer_id) {
            peer_state.status = KnownPeerStatus::NotConnected;
        } else {
            bail!("Peer {} is missing in the peer store", peer_id);
        }
        Ok(())
    }
}

impl PeerStore {
    pub fn new(clock: &time::Clock, config: Config) -> anyhow::Result<Self> {
        let boot_nodes: HashSet<_> = config.boot_nodes.iter().map(|p| p.id.clone()).collect();
        
        // Safely validate and convert cache size to prevent panics
        let cache_size = NonZeroUsize::new(config.peer_states_cache_size as usize)
            .ok_or_else(|| anyhow::anyhow!("peer_states_cache_size configuration must be greater than 0"))?;
            
        let mut peer_id_2_state = LruCache::new(cache_size);
        let mut addr_2_peer = HashMap::default();

        if boot_nodes.len() > (config.peer_states_cache_size as usize) {
            tracing::error!(
                boot_nodes_len = boot_nodes.len(),
                %config.peer_states_cache_size,
                "number of boot nodes exceeds peer store size limit"
            );
        }
        
        // ... (remaining initialization code)

🌐 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