Skip to content

Spammer should reject transaction mix weights whose total overflows #352

Description

@Kewe63

Summary

The spammer accepts individually valid u32 transaction mix weights whose aggregate exceeds u32::MAX.

For example:

--mix transfer=4294967295,legacy=1

passes CLI parsing. The first call to TxTypeMix::total_weight() then overflows its u32 addition and panics in overflow-checking builds. The real spammer CLI reproduces this during Config::validate() with exit code 101.

The aggregate should be validated safely and rejected with a normal --mix error instead of reaching unchecked arithmetic.

Affected files

  • crates/spammer/src/config.rs
  • crates/spammer/src/main.rs
  • crates/spammer/src/generator.rs

Observed behavior

Each --mix weight is parsed independently as u32:

let weight: u32 = raw_value
    .trim()
    .parse()
    .map_err(|_| format!("Invalid weight '{raw_value}' for '{raw_key}'"))?;

The parser then returns the mix without validating its aggregate:

Ok(out)

TxTypeMix::total_weight() performs unchecked u32 addition:

pub fn total_weight(&self) -> u32 {
    self.transfer + self.legacy + self.erc20 + self.guzzler
}

Config::validate() calls this method while checking for an all-zero mix:

if self.tx_type_mix.total_weight() == 0 {
    eyre::bail!("--mix total weight is 0; at least one tx type must have weight > 0");
}

The generator uses the same method when selecting a transaction type:

let total = self.tx_type_mix.total_weight();
let mut pick = rand::thread_rng().gen_range(0..total);

The combination below contains two individually valid values but has an aggregate that does not fit in u32:

transfer=4294967295,legacy=1

Expected behavior

  • Aggregate transaction mix weights should be computed without overflow.
  • A --mix value whose total exceeds the supported aggregate range should be rejected during parsing or configuration validation.
  • The error should identify --mix and explain that the total weight is too large.
  • Programmatically constructed TxTypeMix values should not make Config::validate() panic.
  • Valid ratio-based mixes should remain unchanged.

Reproduction

I added three focused tests against current main.

Parser acceptance

#[test]
fn tx_type_mix_rejects_overflowing_total() {
    let result = TxTypeMix::from_str("transfer=4294967295,legacy=1");
    assert!(
        result.is_err(),
        "--mix parser accepted weights whose total cannot fit in u32"
    );
}

Result:

test config::tests::tx_type_mix_rejects_overflowing_total ... FAILED
--mix parser accepted weights whose total cannot fit in u32

Configuration panic

#[test]
fn config_validation_does_not_panic_on_overflowing_mix_total() {
    let config = Config {
        tx_type_mix: TxTypeMix {
            transfer: u32::MAX,
            legacy: 1,
            ..Default::default()
        },
        ..default_config()
    };

    let result = std::panic::catch_unwind(|| config.validate());
    assert!(result.is_ok(), "overflowing --mix total caused a panic");
    assert!(result.unwrap().is_err(), "overflowing --mix total was accepted");
}

Result:

thread 'config::tests::config_validation_does_not_panic_on_overflowing_mix_total' panicked at crates/spammer/src/config.rs:255:9:
attempt to add with overflow

test config::tests::config_validation_does_not_panic_on_overflowing_mix_total ... FAILED

Focused command:

cargo +1.94.0 test -p spammer overflowing -- --nocapture

Result:

test result: FAILED. 0 passed; 2 failed; 72 filtered out

CLI parsing

A separate CLI regression test confirmed that the same value is accepted as --mix input:

test tests::cli_rejects_overflowing_transaction_mix_total ... FAILED
CLI accepted transaction mix weights whose total overflows u32

The real binary also reproduces the panic:

spammer --silent --mix transfer=4294967295,legacy=1 ws

Result:

thread 'main' panicked at crates/spammer/src/config.rs:255:9:
attempt to add with overflow

Exit code:

101

The panic occurs during configuration validation, before a WebSocket connection is attempted.

Tested against commit:

97f8da0dc4faa703fe2d68ca007e40dab2c8a9ef

Root cause

The input boundary validates each weight independently as u32, but never validates the sum. Downstream code assumes the sum also fits in u32 and uses ordinary addition.

This creates a cross-field invariant that is neither checked nor represented safely in the aggregate calculation.

Why this matters

Invalid user-controlled CLI input terminates the process with a panic after parsing has reported success. Scripts and orchestrators receive an internal overflow failure instead of a predictable argument/configuration error.

The same unchecked total is also used by transaction-type selection, so the invariant should be enforced before generation begins rather than relying on build-profile overflow behavior.

Suggested fix

Validate the aggregate without performing overflowing u32 arithmetic.

One focused approach is to:

  1. compute the total using checked addition or a wider intermediate type;
  2. reject totals above u32::MAX with a descriptive --mix error;
  3. enforce the same invariant in Config::validate() for programmatically constructed or deserialized values;
  4. ensure generator selection cannot call an overflowing total_weight() implementation.

Avoid saturating or wrapping arithmetic because either would silently change the configured ratios.

Potential regression tests

Add boundary tests verifying that:

  • ordinary mixes such as transfer=70,legacy=30 remain valid;
  • a total equal to u32::MAX is accepted;
  • transfer=4294967295,legacy=1 is rejected;
  • overflow across later fields is also rejected;
  • Config::validate() returns an error rather than panicking for a programmatically constructed overflowing mix;
  • the real CLI exits with a normal error and no panic text.

Scope

This report concerns the transaction-type weights supplied through --mix and represented by TxTypeMix.

It does not require changes to transaction generation, rate limiting, WebSocket behavior, or consensus/protocol behavior beyond making transaction-type selection consume a safely validated aggregate.

Duplicate check

I searched open and closed issues and pull requests using combinations of:

  • spammer transaction mix overflow
  • transfer=4294967295
  • total_weight
  • --mix overflow
  • attempt to add with overflow
  • u32::MAX

No direct duplicate or existing implementation was found.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions