[RWF] relay-substrate-client: transaction_stall_timeout overflows near max mortality

[xlc]

Summary

transaction_stall_timeout computes mortality_period + 1 + 1 using regular addition on a u32. For mortality_period values >= u32::MAX - 1, this overflows and panics in debug builds (and silently wraps in release). The function already uses saturating_mul on the same line, so the inconsistent use of plain + appears to be an oversight. The function is a public API that accepts any u32 and is called with unvalidated TransactionParams::mortality values.

Severity / Sensitivity

Low severity, non-sensitive public issue. This is reported from a confirmed Runtime Whitebox Fuzzer finding against public Polkadot SDK code.

Source Evidence

• bridges/relays/client-substrate/src/lib.rs lines 84-87: The function body mixes saturating_mul with plain + 1 + 1 on the mortality_period. The non-saturating addition overflows for mortality_period >= u32::MAX - 1, contradicting the saturating intent shown by saturating_mul on the same line.
• bridges/relays/lib-substrate-relay/src/lib.rs lines 39-43: TransactionParams exposes mortality as an Option<u32>, so code outside the CLI parser can construct values near u32::MAX unless the lower-level timeout function validates or saturates them.
• bridges/relays/lib-substrate-relay/src/finality/mod.rs lines 266-270: The function is called directly with transaction_params.mortality — the unbounded Option<u32> from TransactionParams — without any additional validation before reaching the overflow-prone arithmetic.
• bridges/relays/lib-substrate-relay/src/cli/chain_schema.rs lines 152-175: CLI documentation states mortality 'MUST be a power of two in [4; 65536] range', and the CLI enforces this. However, this validation is at the CLI parsing layer only — the function itself and the TransactionParams struct enforce no such bound, so non-CLI callers (or programmatic construction) can pass arbitrary u32 values.

Full Relevant PoC Test

Paste the snippet below into an in-crate unit test module for the affected package. It is self-contained apart from helpers and types already available in that crate's existing test context.

use super::*;

use std::time::Duration;

fn stall_timeout_saturating_reference(
	mortality_period: Option<u32>,
	average_block_interval: Duration,
	default_stall_timeout: Duration,
) -> Duration {
	mortality_period
		.map(|mortality_period| {
			average_block_interval.saturating_mul(mortality_period.saturating_add(2))
		})
		.unwrap_or(default_stall_timeout)
}

// RWF-RISKS: risk_6eq9x75e
// RWF-INVARIANTS: inv_tkj4592h
#[test]
fn test_risk_6eq9x75e_no_panic_at_mortality_near_max() {
	let interval = Duration::from_secs(6);
	let default = Duration::from_secs(600);
	let expected = stall_timeout_saturating_reference(Some(u32::MAX), interval, default);

	let actual = transaction_stall_timeout(Some(u32::MAX), interval, default);

	assert_eq!(
		actual,
		expected,
		"transaction_stall_timeout must not overflow or wrap near u32::MAX",
	);
}

Reproduction Command

cargo test -p relay-substrate-client test_risk_6eq9x75e_no_panic_at_mortality_near_max -- --nocapture

Observed Result

The PoC fails on the current implementation with:

attempt to add with overflow

Expected Behavior

The target should preserve the invariant described above instead of producing the confirmed failure. In particular, the affected operation should reject malformed or inconsistent input, preserve durable state invariants, or return an error rather than silently corrupting state or panicking.

Validation Rationale

The public source shows transaction_stall_timeout accepting Option<u32> and computing mortality_period + 1 + 1 before applying saturating_mul. TransactionParams::mortality is also an Option<u32>, and the call site forwards it directly. The PoC passes u32::MAX, compares the result against a saturating reference, and the current implementation panics during the unchecked addition.

RWF Metadata

• Found by Runtime Whitebox Fuzzer
• Finding: find_it3qysfhwb
• Report: freport_ry3m5i7r6k
• Target: relay-substrate-client
• Run: 20260628-212807-a53980
• Severity: low
• Category: Arithmetic overflow / panic on large input to public function
• Invariants: inv_tkj4592h
• Risks: risk_6eq9x75e