[RWF] polkadot-erasure-coding: reconstruct panics on out-of-bounds chunk index

[xlc]

Summary

reconstruct allocates received_shards with length n_validators, then directly indexes it with caller-provided chunk_idx. A chunk index >= n_validators panics instead of returning the existing Error::ChunkIndexOutOfBounds variant.

Severity / Sensitivity

Medium severity, non-sensitive public issue. This is an input-validation panic in erasure-coding reconstruction.

Source Evidence

• polkadot/erasure-coding/src/lib.rs lines 223-240 build received_shards and index it with received_shards[chunk_idx] without a bounds check.
• polkadot/erasure-coding/src/lib.rs lines 66-68 define Error::ChunkIndexOutOfBounds, which matches this failure mode.

PoC Unit Test

use super::*;

use std::sync::Arc;

use polkadot_node_primitives::{BlockData, PoV};
use polkadot_primitives::{HeadData, PersistedValidationData};

// ---------------------------------------------------------------------------
// Local setup helpers
// ---------------------------------------------------------------------------

/// Build a deterministic, non-empty `AvailableData` whose PoV body is
/// `pov_byte_count` bytes long. The body length is clamped to a minimum of 1
/// so the SCALE encoding is never empty (an empty payload is rejected by
/// `obtain_chunks` with `Error::BadPayload`).
fn make_available_data(pov_byte_count: usize) -> AvailableData {
	AvailableData {
		pov: Arc::new(PoV { block_data: BlockData(vec![0xAA; pov_byte_count.max(1)]) }),
		validation_data: PersistedValidationData {
			parent_head: HeadData(vec![1, 2, 3, 4]),
			relay_parent_number: 42,
			relay_parent_storage_root: Default::default(),
			max_pov_size: 2048,
		},
	}
}

/// Drive several out-of-bounds chunk indices through `reconstruct`.
///
/// `reconstruct` (lib.rs) builds `received_shards: Vec<Option<_>>` of length
/// `n_validators` and then indexes it with `received_shards[chunk_idx]` without
/// first checking `chunk_idx < n_validators`. The `.take(n_validators)` call on
/// the iterator only limits how many chunks are consumed, not the magnitude of
/// their indices, so any index `>= n_validators` escapes the vector and panics.
///
/// This helper asserts each out-of-bounds index is reported as an `Err`
/// (`Error::ChunkIndexOutOfBounds`) rather than panicking. On the current
/// implementation the panic propagates, so tests that call this helper are
/// marked `#[ignore]` until the missing bounds check is added.
fn assert_reconstruct_no_oob_access(n_validators: usize, valid_chunks: &[Vec<u8>]) {
	if valid_chunks.len() < 2 {
		return;
	}
	for &oob_idx in &[n_validators, n_validators + 1, n_validators + 10] {
		let result = std::panic::catch_unwind(std::panic::AssertUnwindSafe(|| {
			reconstruct::<_, Vec<u8>>(
				n_validators,
				vec![(&valid_chunks[0][..], 0), (&valid_chunks[1][..], oob_idx)],
			)
		}));
		match result {
			Ok(Ok(_)) =>
				panic!("reconstruct accepted out-of-bounds index {} without error", oob_idx),
			Ok(Err(_)) => {},
			Err(_) => panic!(
				"reconstruct panicked on out-of-bounds index {} (should return error instead)",
				oob_idx
			),
		}
	}
}

/// Encode `data` with `obtain_chunks`, then reconstruct it from a
/// `recovery_threshold(n)`-sized, correctly-indexed subset of the produced
/// chunks. The recovered value must equal the original exactly.
fn assert_round_trip_encode_decode<T>(n_validators: usize, data: &T)
where
	T: Encode + Decode + PartialEq + Clone + std::fmt::Debug,
{
	let chunks = obtain_chunks(n_validators, data).expect("obtain_chunks should succeed");
	assert_eq!(chunks.len(), n_validators, "obtain_chunks must yield n_validators shards");

	let threshold =
		recovery_threshold(n_validators).expect("recovery_threshold should succeed");

	let subset: Vec<(&[u8], usize)> = chunks
		.iter()
		.enumerate()
		.take(threshold)
		.map(|(i, c)| (c.as_slice(), i))
		.collect();

	let recovered: T = reconstruct(n_validators, subset)
		.expect("reconstruct should succeed with a threshold-sized subset");

	assert_eq!(&recovered, data, "round-trip encode/decode must recover the original data");
}

// ---------------------------------------------------------------------------
// Risk coverage: `reconstruct` indexes `received_shards[chunk_idx]` directly
// (lib.rs) without validating `chunk_idx < n_validators`. The
// `Error::ChunkIndexOutOfBounds` variant is defined but never produced, so any
// index >= n_validators panics with an index-out-of-bounds instead of being
// reported as an error.
// ---------------------------------------------------------------------------

// RWF-INVARIANTS: inv_cxgqx66u,inv_yb78ftg9
// RWF-RISKS: risk_ck74458z,risk_z44pk8tv,risk_z9vrug6w
#[ignore]
#[test]
fn test_risk_ck74458z_reconstruct_oob_index() {
	// A chunk index equal to (or greater than) `n_validators` lands outside the
	// `received_shards` vector. `reconstruct` should return
	// `Error::ChunkIndexOutOfBounds`, but currently panics with an
	// index-out-of-bounds. Ignored until the missing bounds check is added.
	let chunks = obtain_chunks(10, &make_available_data(64)).unwrap();
	assert_eq!(chunks.len(), 10);
	assert_reconstruct_no_oob_access(10, &chunks);
}

// RWF-INVARIANTS: inv_imtcybrn,inv_2nky54ie
// RWF-RISKS: risk_g93xkbex

Reproduction Command

cargo test -p polkadot-erasure-coding generated_tests::test_risk_ck74458z_reconstruct_oob_index -- --ignored --exact --nocapture

Observed Result

The ignored negative regression catches `reconstruct` panicking on an out-of-bounds chunk index. RWF validation recorded this as an expected-failing PoC because the current implementation panics instead of returning `ChunkIndexOutOfBounds`.

Expected Behavior

reconstruct should validate chunk_idx < n_validators and return Error::ChunkIndexOutOfBounds for invalid indices instead of panicking.

Likely Fix Direction

Check the chunk index before writing into received_shards, and return the existing ChunkIndexOutOfBounds error variant.

RWF Metadata

• Found by Runtime Whitebox Fuzzer
• Finding: find_hdegrar5ei
• Target: polkadot-erasure-coding
• Run: 20260609-092921-4a8dad
• Source commit: b95ef390bb5b4c3025e244234373195e4d1c12ed
• Severity: medium
• Category: input-validation
• Invariants: inv_yb78ftg9
• Risks: risk_ck74458z, risk_z44pk8tv, risk_z9vrug6w

[talhadaar]

Hey @xlc, i'd like to work on this if not already taken.