From 86cc3977a2d33dc606ead42e4505bbe6a11c018f Mon Sep 17 00:00:00 2001 From: Roman Date: Fri, 26 Jun 2026 14:56:29 +0800 Subject: [PATCH] fix: exclude reserved system IDs --- .../287c1358afcc2c83b86a03bd1f06e33811037527 | Bin 0 -> 200 bytes .../957d6b0a369114d7315e4a379495636beabe3e5b | Bin 0 -> 319 bytes .../d2d347de29031b8ed90521f782aa96ec8bf3e83c | Bin 0 -> 198 bytes fuzz_props/src/generators.rs | 57 ++++++++++--- fuzz_props/src/tests/generators.rs | 75 ++++++++++++++++-- 5 files changed, 116 insertions(+), 16 deletions(-) create mode 100644 corpus/libfuzz/fuzz_multi_block_state_sequence/287c1358afcc2c83b86a03bd1f06e33811037527 create mode 100644 corpus/libfuzz/fuzz_multi_block_state_sequence/957d6b0a369114d7315e4a379495636beabe3e5b create mode 100644 corpus/libfuzz/fuzz_multi_block_state_sequence/d2d347de29031b8ed90521f782aa96ec8bf3e83c diff --git a/corpus/libfuzz/fuzz_multi_block_state_sequence/287c1358afcc2c83b86a03bd1f06e33811037527 b/corpus/libfuzz/fuzz_multi_block_state_sequence/287c1358afcc2c83b86a03bd1f06e33811037527 new file mode 100644 index 0000000000000000000000000000000000000000..2d997a2593ee4dd9bcba3c7d8965f2d6a84ced3f GIT binary patch literal 200 zcmZ1|W+dSErX}EAU*S)=$W@m^TpqgHiTv_Vez*4D>3Gqd$umG;A>%?Im@(sjIvC8z zNKa>AU|vJK9Ck<;D9LnKSKhn7ObQL!UP%$V&q<&k-!Ks glLbuuW=AsNf5w~{aX?k^8R;|Nh5#)F>H5zI0M>SJEC2ui literal 0 HcmV?d00001 diff --git a/corpus/libfuzz/fuzz_multi_block_state_sequence/957d6b0a369114d7315e4a379495636beabe3e5b b/corpus/libfuzz/fuzz_multi_block_state_sequence/957d6b0a369114d7315e4a379495636beabe3e5b new file mode 100644 index 0000000000000000000000000000000000000000..3e46c47cb4f2dbd00ddec7847160895d84396e86 GIT binary patch literal 319 zcmZQ%a9|(|{{R2~2d}CyxU86wfZv;zfOma`Kjk7Lf83O+QZw4_QL%Co|ADII*OB(<=P;R~e literal 0 HcmV?d00001 diff --git a/fuzz_props/src/generators.rs b/fuzz_props/src/generators.rs index ca79628c..f1b6949d 100644 --- a/fuzz_props/src/generators.rs +++ b/fuzz_props/src/generators.rs @@ -59,19 +59,54 @@ pub struct FuzzAccount { /// conservation checks can therefore use `checked_add` instead of `saturating_add` to /// turn silent overflow into a detected violation, ruling out false-positive panics on /// legitimate fuzz inputs. +/// +/// # Reserved-ID and duplicate exclusion +/// +/// The cap above is only sound if every generated balance survives genesis construction +/// unchanged. Two failure modes break that: +/// +/// * **Reserved system accounts.** [`nssa::V03State::new_with_genesis_accounts`] inserts +/// the faucet account (`balance = u128::MAX`) and bridge account *after* the supplied +/// genesis accounts, overwriting any generated account whose ID collides. A fuzzer that +/// lands on the faucet ID would make a caller read back `u128::MAX` instead of the capped +/// balance it generated, overflowing the conservation sum — a harness false positive, not +/// a protocol bug. +/// * **Duplicate IDs.** Genesis stores accounts in a `HashMap` keyed by ID, so duplicate +/// IDs collapse to a single (last-write-wins) account, while a caller's per-ID balance sum +/// double-counts that account's balance. +/// +/// Both are excluded here: generated IDs equal to a reserved system account, or already +/// seen in this state, are skipped. The result therefore contains only distinct, +/// non-reserved IDs whose generated balances match what genesis stores — so `0..=8` +/// accounts are returned (an empty state is a valid degenerate case). pub fn arbitrary_fuzz_state(u: &mut Unstructured<'_>) -> arbitrary::Result> { + let reserved = [ + nssa::system_faucet_account_id(), + nssa::system_bridge_account_id(), + ]; let n = ((u8::arbitrary(u)? as usize) % 8) + 1; // 1..=8 - std::iter::repeat_with(|| { - Ok(FuzzAccount { - account_id: ArbAccountId::arbitrary(u)?.0, - // Divide by 8 so the sum of 8 accounts is at most u128::MAX, preventing - // false-positive checked_add panics that would mask real inflation bugs. - balance: u128::arbitrary(u)? / 8, - private_key: ArbPrivateKey::arbitrary(u)?.0, - }) - }) - .take(n) - .collect() + + let mut seen = std::collections::HashSet::with_capacity(n); + let mut accounts = Vec::with_capacity(n); + for _ in 0..n { + let account_id = ArbAccountId::arbitrary(u)?.0; + // Divide by 8 so the sum of 8 accounts is at most u128::MAX, preventing + // false-positive checked_add panics that would mask real inflation bugs. + let balance = u128::arbitrary(u)? / 8; + let private_key = ArbPrivateKey::arbitrary(u)?.0; + + // Skip IDs that genesis would overwrite (reserved system accounts) or that would + // collapse on insertion (duplicates); see the doc comment above. + if reserved.contains(&account_id) || !seen.insert(account_id) { + continue; + } + accounts.push(FuzzAccount { + account_id, + balance, + private_key, + }); + } + Ok(accounts) } /// Reduce raw fuzzer draws into a *biased-valid* `(nonce, amount)` pair. diff --git a/fuzz_props/src/tests/generators.rs b/fuzz_props/src/tests/generators.rs index ec5584b5..b3f2cfce 100644 --- a/fuzz_props/src/tests/generators.rs +++ b/fuzz_props/src/tests/generators.rs @@ -49,22 +49,37 @@ fn signer_ids_contains_the_signing_account() { ); } +/// A buffer whose bytes are all distinct within any 80-byte window (the per-account +/// stride: 32 id + 16 balance + 32 key), so each generated account gets a distinct ID +/// and the dedup pass in `arbitrary_fuzz_state` does not collapse the count. Using +/// `buf[i] = i` works because two account-ID windows starting at offsets `a` and `b` +/// (both `< 256`) are equal only when `a ≡ b (mod 256)`, which never holds for the +/// `1 + j*80` offsets of the first eight accounts. +fn distinct_byte_buffer(len: usize) -> Vec { + (0_u8..=255).cycle().take(len).collect() +} + #[test] -fn fuzz_state_never_empty() { - let buf = vec![0_u8; 1000]; +fn fuzz_state_never_empty_for_distinct_ids() { + // Selector byte 0 -> (0 % 8) + 1 = 1 account; distinct bytes keep it from being + // deduped away. (An all-duplicate or all-reserved draw may legitimately return + // 0 accounts now — see `fuzz_state_dedups_account_ids` — so non-emptiness is only + // asserted for an input that yields distinct, non-reserved IDs.) + let buf = distinct_byte_buffer(1000); let mut u = Unstructured::new(&buf); let accounts = arbitrary_fuzz_state(&mut u).expect("should succeed"); assert!( !accounts.is_empty(), - "arbitrary_fuzz_state must return at least 1 account (n = 1..=8); \ + "arbitrary_fuzz_state must return at least 1 account for distinct-ID input; \ returned 0 \u{2014} mutation: `+ 1` replaced by `* 1` or `Ok(vec![])`" ); } #[test] fn fuzz_state_count_uses_modulo_not_div_or_add() { - // fill_buffer reads from the front; the first byte is the n-selector. - let mut buf = vec![0_u8; 1000]; + // fill_buffer reads from the front; the first byte is the n-selector. Distinct + // bytes give every account a unique ID so the count is not masked by dedup. + let mut buf = distinct_byte_buffer(1000); buf[0] = 8; // selector byte: 8 % 8 = 0, +1 -> n=1 | 8 / 8 = 1, +1 -> n=2 | 8 + 8 = 16, +1 -> n=17 let mut u = Unstructured::new(&buf); let accounts = arbitrary_fuzz_state(&mut u).expect("should succeed"); @@ -76,6 +91,56 @@ fn fuzz_state_count_uses_modulo_not_div_or_add() { ); } +#[test] +fn fuzz_state_excludes_reserved_system_ids() { + // Genesis overwrites the faucet (balance = u128::MAX) and bridge accounts after + // inserting the supplied genesis accounts; a generated account colliding with one + // would read back a balance the cap never produced, overflowing conservation sums. + // The generator must therefore never emit a reserved system ID. + let reserved = [ + nssa::system_faucet_account_id(), + nssa::system_bridge_account_id(), + ]; + let buf = distinct_byte_buffer(10_000); + let mut u = Unstructured::new(&buf); + let accounts = arbitrary_fuzz_state(&mut u).expect("should succeed"); + for acc in &accounts { + assert!( + !reserved.contains(&acc.account_id), + "arbitrary_fuzz_state emitted reserved system account ID {:?} \u{2014} \ + genesis would overwrite it and break the balance-conservation invariant", + acc.account_id + ); + } +} + +#[test] +fn fuzz_state_dedups_account_ids() { + // All-identical bytes make every drawn account ID identical; genesis stores + // accounts in a HashMap (last-write-wins), so duplicate IDs would let a per-ID + // balance sum double-count one account. The generator must collapse them to one. + let buf = vec![0xAB_u8; 10_000]; + let mut u = Unstructured::new(&buf); + let accounts = arbitrary_fuzz_state(&mut u).expect("should succeed"); + assert!( + accounts.len() <= 1, + "arbitrary_fuzz_state must dedup identical account IDs; got {} accounts", + accounts.len() + ); + + // Independent confirmation on a distinct-ID draw: no ID appears twice. + let distinct_buf = distinct_byte_buffer(10_000); + let mut distinct_u = Unstructured::new(&distinct_buf); + let distinct_accounts = arbitrary_fuzz_state(&mut distinct_u).expect("should succeed"); + let unique: std::collections::HashSet<_> = + distinct_accounts.iter().map(|a| a.account_id).collect(); + assert_eq!( + unique.len(), + distinct_accounts.len(), + "arbitrary_fuzz_state returned duplicate account IDs" + ); +} + /// Verifies that each account's balance is <= `u128::MAX / 8`. #[test] fn fuzz_state_balances_bounded_by_max_div_8() {