Summary

finalize_root_at (line 450) unconditionally assigns best_finalized_number = Some(node.number) without verifying the new value exceeds the current one. finalize_with_descendent_if creates a valid state where a root with a low block number survives below best_finalized_number (retained via the ancestor clause at line 696 while best_finalized_number is advanced at line 705). In that state, calling finalize (or the public finalize_root directly) on the low-numbered root resets best_finalized_number to the root's lower number, violating the struct's guarantee that 'nodes in the tree are finalized in order' (line 83) and the Revert error contract (lines 33-34). The revert guard in finalize uses the caller-supplied number rather than the matched root's stored number, allowing the guard to be bypassed. Unlike finalize_with_descendent_if (which overwrites best_finalized_number at line 705 after any root finalization), finalize and finalize_with_ancestors return immediately after finalize_root_at with no corrective overwrite.

Severity / Sensitivity

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

Source Evidence

• substrate/utils/fork-tree/src/lib.rs lines 447-452: finalize_root_at unconditionally sets best_finalized_number = Some(node.number) with no check that node.number exceeds the current value, enabling a monotonicity violation when a root exists below the finalized level.
• substrate/utils/fork-tree/src/lib.rs lines 469-478: The revert guard in finalize checks the caller-provided number (line 470), but when a root matches by hash (line 476), finalize_root_at uses the root's own stored number for best_finalized_number, creating a mismatch that allows the guard to be bypassed.
• substrate/utils/fork-tree/src/lib.rs lines 693-705: finalize_with_descendent_if's third retain clause (line 696) keeps ancestor roots whose number is below the finalized block, while line 705 sets best_finalized_number to the higher finalized number — creating the vulnerable state where a root exists below the finalized level.
• substrate/utils/fork-tree/src/lib.rs lines 79-86: The struct documentation states 'It also guarantees that nodes in the tree are finalized in order', establishing the monotonicity invariant that best_finalized_number should never decrease.
• substrate/utils/fork-tree/src/lib.rs lines 1176-1183: The existing test finalize_with_descendent_works confirms that retaining a root (A0 at number 1) below best_finalized_number (2) is expected behavior after a finalize_with_descendent_if call with a failing predicate — establishing that the vulnerable precondition is reachable through normal API usage.
• substrate/utils/fork-tree/src/lib.rs lines 439-444: finalize_root is a public method with no revert guard; calling it directly on a root whose number is below best_finalized_number would also decrease best_finalized_number via finalize_root_at, confirming the bug is reachable without any number mismatch.

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::*;

#[derive(Debug, PartialEq)]
struct TestError;

impl std::fmt::Display for TestError {
	fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
		write!(f, "test ancestry error")
	}
}

impl std::error::Error for TestError {}

fn assert_best_finalized_number_never_decreases<H, N, V>(
	before: &ForkTree<H, N, V>,
	after: &ForkTree<H, N, V>,
) where
	H: PartialEq,
	N: Ord + Clone + PartialEq + std::fmt::Debug,
	V: PartialEq,
{
	match (&before.best_finalized_number, &after.best_finalized_number) {
		(Some(prev), Some(curr)) => assert!(
			curr >= prev,
			"best_finalized_number decreased from {:?} to {:?}",
			prev,
			curr,
		),
		(None, Some(_)) | (None, None) => {},
		(Some(prev), None) => panic!(
			"best_finalized_number went from Some({:?}) to None; it must never decrease",
			prev,
		),
	}
}

const LOW_ROOT_HASH: u64 = 0xA0;
const FINAL_HASH: u64 = 0xA1;
const LOW_ROOT_NUMBER: u64 = 5;
const FINAL_NUMBER: u64 = 10;

fn is_desc_of_planted(base: &u64, target: &u64) -> Result<bool, TestError> {
	Ok(*base == LOW_ROOT_HASH && *target == FINAL_HASH)
}

fn build_tree_with_low_root_below_finalized() -> ForkTree<u64, u64, u64> {
	let mut tree: ForkTree<u64, u64, u64> = ForkTree::new();

	tree.import(LOW_ROOT_HASH, LOW_ROOT_NUMBER, 100, &is_desc_of_planted).unwrap();
	assert_eq!(tree.best_finalized_number, None);

	let res = tree.finalize_with_descendent_if(
		&FINAL_HASH,
		FINAL_NUMBER,
		&is_desc_of_planted,
		|_| false,
	);
	assert_eq!(res.unwrap(), FinalizationResult::Unchanged);
	assert_eq!(tree.best_finalized_number, Some(FINAL_NUMBER));

	let roots: Vec<(u64, u64)> = tree.roots().map(|(h, n, _)| (*h, *n)).collect();
	assert_eq!(roots, vec![(LOW_ROOT_HASH, LOW_ROOT_NUMBER)]);

	tree
}

// RWF-RISKS: risk_wa59cxt4
// RWF-INVARIANTS: inv_8gqkrath
#[test]
fn test_risk_wa59cxt4_finalize_root_decreases_best_finalized_number() {
	let mut tree = build_tree_with_low_root_below_finalized();
	let before = tree.clone();
	assert_eq!(tree.best_finalized_number, Some(FINAL_NUMBER));

	let data = tree.finalize_root(&LOW_ROOT_HASH);
	assert_eq!(data, Some(100));
	assert!(tree.roots.is_empty());

	assert_best_finalized_number_never_decreases(&before, &tree);
	assert_eq!(tree.best_finalized_number, Some(FINAL_NUMBER));
}

Reproduction Command

cargo test -p fork-tree test_risk_wa59cxt4_finalize_root_decreases_best_finalized_number -- --nocapture

Observed Result

The PoC fails on the current implementation with:

best_finalized_number decreased from 10 to 5

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

All technical claims are verified against the public source evidence above. finalize_root_at unconditionally assigns best_finalized_number = Some(node.number), while finalize_with_descendent_if can leave a lower-numbered ancestor root in the tree after advancing best_finalized_number to a higher finalized block. The PoC constructs that state through public APIs, calls finalize_root on the surviving low-numbered root, and asserts the finalized watermark never decreases. The assertion fails with the observed 10-to-5 regression.

RWF Metadata

• Found by Runtime Whitebox Fuzzer
• Finding: find_ck8ymi3rhj
• Report: freport_i2x9y6disy
• Target: fork-tree
• Run: 20260628-212807-a53980
• Severity: medium
• Category: logic
• Invariants: inv_8gqkrath
• Risks: risk_wa59cxt4