fix: replace saturating_add to prevent overflows

This commit is contained in:
Roman 2026-05-28 11:52:10 +08:00
parent 88e780b865
commit 3c8844068f
No known key found for this signature in database
GPG Key ID: 583BDF43C238B83E
4 changed files with 65 additions and 8 deletions

View File

@ -57,7 +57,12 @@ fuzz_props::fuzz_entry!(|data: &[u8]| {
let starting_total: u128 = init_accs
.iter()
.map(|&(id, _)| state.get_account_by_id(id).balance)
.fold(0u128, u128::saturating_add);
.try_fold(0u128, |acc, x| acc.checked_add(x))
.expect(
"INVARIANT VIOLATION [BalanceOverflow]: initial sum of genesis account balances \
exceeded u128::MAX per-account balance cap in arbitrary_fuzz_state() should \
prevent this; if triggered, the cap has been raised without updating this check",
);
// Apply up to 16 transactions across successive blocks.
let n_txs: u8 = u8::arbitrary(&mut u).unwrap_or(0) % 16;
@ -119,7 +124,12 @@ fuzz_props::fuzz_entry!(|data: &[u8]| {
let ending_total: u128 = init_accs
.iter()
.map(|&(id, _)| state.get_account_by_id(id).balance)
.fold(0u128, u128::saturating_add);
.try_fold(0u128, |acc, x| acc.checked_add(x))
.expect(
"INVARIANT VIOLATION [BalanceOverflow]: final sum of genesis account balances \
exceeded u128::MAX token-inflation bug that saturating_add would have \
silently masked",
);
assert_eq!(
starting_total,

View File

@ -141,11 +141,21 @@ fuzz_props::fuzz_entry!(|data: &[u8]| {
let total_before: u128 = known_ids
.iter()
.map(|id| state.get_account_by_id(*id).balance)
.fold(0u128, u128::saturating_add);
.try_fold(0u128, |acc, x| acc.checked_add(x))
.expect(
"INVARIANT VIOLATION [BalanceOverflow]: pre-execution sum of known account \
balances exceeded u128::MAX token-inflation bug that saturating_add would \
have silently masked",
);
let total_after: u128 = known_ids
.iter()
.map(|id| exec_state.get_account_by_id(*id).balance)
.fold(0u128, u128::saturating_add);
.try_fold(0u128, |acc, x| acc.checked_add(x))
.expect(
"INVARIANT VIOLATION [BalanceOverflow]: post-execution sum of known account \
balances exceeded u128::MAX token-inflation bug that saturating_add would \
have silently masked",
);
assert_eq!(
total_before,
total_after,

View File

@ -51,12 +51,22 @@ pub struct FuzzAccount {
///
/// Call this before generating transactions so the constructed [`nssa::V03State`]
/// has a shape controlled by the fuzzer rather than fixed at compile time.
///
/// # Balance cap
///
/// Each account's balance is capped at `u128::MAX / 8`. With at most 8 accounts, this
/// guarantees the sum of all balances fits in a `u128` without overflow. Balance-
/// 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.
pub fn arbitrary_fuzz_state(u: &mut Unstructured<'_>) -> arbitrary::Result<Vec<FuzzAccount>> {
let n = ((u8::arbitrary(u)? as usize) % 8) + 1; // 1..=8
std::iter::repeat_with(|| {
Ok(FuzzAccount {
account_id: ArbAccountId::arbitrary(u)?.0,
balance: u128::arbitrary(u)?,
// 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,
})
})

View File

@ -7,9 +7,25 @@ use nssa_core::account::Nonce;
pub struct BalanceSnapshot(pub std::collections::HashMap<nssa::AccountId, u128>);
impl BalanceSnapshot {
/// Capture current total balance over all known accounts.
/// Sum of all recorded account balances.
///
/// # Panics
///
/// Panics if the sum overflows `u128`. This indicates a token-inflation bug — i.e.
/// the protocol somehow created tokens past `u128::MAX` — and would have been silently
/// masked by `saturating_add`. The generator caps each account balance at
/// `u128::MAX / 8` so eight accounts never overflow; any overflow here is therefore
/// a genuine protocol violation, not a fuzzer artefact.
#[must_use]
pub fn total(&self) -> u128 {
self.0.values().copied().fold(0_u128, u128::saturating_add)
self.0
.values()
.copied()
.try_fold(0_u128, u128::checked_add)
.expect(
"INVARIANT VIOLATION [BalanceOverflow]: sum of account balances exceeded u128::MAX \
\u{2014} token-inflation bug that saturating_add would have silently masked",
)
}
}
@ -97,6 +113,12 @@ impl ProtocolInvariant for BalanceConservation {
"BalanceConservation"
}
// Overflow in the balance sum IS the violation; using `?` here would silently return
// `None` and skip the check, which is worse than the inflation bug it was meant to catch.
#[expect(
clippy::unwrap_in_result,
reason = "overflow panic is the intended signal"
)]
fn check(&self, ctx: &InvariantCtx<'_>) -> Option<InvariantViolation> {
if ctx.execution_succeeded {
let total_before = ctx.balances_before.total();
@ -105,7 +127,12 @@ impl ProtocolInvariant for BalanceConservation {
.0
.keys()
.map(|&id| ctx.state_after.get_account_by_id(id).balance)
.fold(0_u128, u128::saturating_add);
.try_fold(0_u128, u128::checked_add)
.expect(
"INVARIANT VIOLATION [BalanceOverflow]: sum of post-execution account balances \
exceeded u128::MAX \u{2014} token-inflation bug that saturating_add would \
have silently masked",
);
if total_before != total_after {
return Some(InvariantViolation {
invariant: self.name(),