Potential overflow in celestia_block_variance to usize conversion
Description
A potential overflow occurs when converting a u64 to usize for the celestia_block_variance.
Impact
The issue arises from trying to convert celestia_block_variance (which is a u64) to usize. On 32-bit systems, this conversion could lead to a panic if celestia_block_variance exceeds u32::MAX. Although unlikely, this could result in a DOS if the system encounters a value larger than u32 during the conversion.
Currently, Conductor only compiles on 64-bit architectures due to restrictions in astria-core, where a compile-time assertion prevents 32-bit system compatibility. The calculate_max_spread function still contains an incorrect expect message regarding the conversion, which could lead to confusion.
/// Calculates the maximum allowed spread between firm and soft commitments heights.
///
/// The maximum allowed spread is taken as `max_spread = variance * 6`, where `variance`
/// is the `celestia_block_variance` as defined in the rollup node's genesis that this
/// executor/conductor talks to.
///
/// The heuristic 6 is the largest number of Sequencer heights that will be found at
/// one Celestia height.
///
/// # Panics
/// Panics if the `u32` underlying the celestia block variance tracked in the state could
/// not be converted to a `usize`. This should never happen on any reasonable architecture
/// that Conductor will run on.
fn calculate_max_spread(&self) -> usize {
usize::try_from(self.state.celestia_block_variance())
.expect("converting a u32 to usize should work on any architecture conductor runs on")
.saturating_mul(6)
}
pub struct GenesisInfo {
/// The rollup id which is used to identify the rollup txs.
rollup_id: RollupId,
/// The Sequencer block height which contains the first block of the rollup.
sequencer_genesis_block_height: tendermint::block::Height,
/// The allowed variance in the block height of celestia when looking for sequencer blocks.
celestia_block_variance: u64,
}Recommendations
We recommend the following:
Update the expect message to correctly describe the
u64-to-usizeconversion.Add a compile-time check within Conductor itself to ensure it only runs on 64-bit systems.