Skip to content

Additional quinn-proto robustness panic candidates #2702

Description

@xc01

I found several generated quinn-proto panic sites. The strongest ones are unchecked arithmetic/state handling in token logging and congestion accounting. Some of the other generated cases depend on custom crypto implementations or invalid enum values, so I would keep those as lower-priority notes rather than lead with them.

I checked nearby comments/docs around the main sites below and did not find # Panics coverage for these conditions.

Version checked: quinn-proto 0.11.14

Main examples

Area Panic site Trigger
Token replay log src/bloom_token_log.rs:74, then src/bloom_token_log.rs:70 Extreme issued + lifetime overflows while the mutex is held, poisoning the token log; the next validation panics on lock().unwrap().
NewReno congestion control src/congestion/new_reno.rs:58 A very large ACK byte count overflows self.window += bytes.
BBR congestion control src/congestion/bbr/bw_estimation.rs:57, src/congestion/bbr/mod.rs:409, src/congestion/bbr/mod.rs:578 Extreme ACK accounting values can overflow internal counters.

Example 1: BloomTokenLog can poison itself on extreme token lifetime

Relevant code:

let mut guard = self.0.lock().unwrap();
let state = &mut *guard;

// calculate how many periods past period 1 the token expires
let expires_at = issued + lifetime;

Reproducer shape:

use std::panic::{catch_unwind, AssertUnwindSafe};
use std::time::{Duration, SystemTime};
use quinn_proto::{BloomTokenLog, TokenLog};

#[test]
#[should_panic]
fn bloom_token_log_extreme_lifetime_poison_then_panics() {
    let log = BloomTokenLog::new_expected_items(1024, 16);

    let first = catch_unwind(AssertUnwindSafe(|| {
        let _ = log.check_and_insert(1, SystemTime::now(), Duration::from_secs(u64::MAX));
    }));
    assert!(first.is_err());

    let _ = log.check_and_insert(2, SystemTime::now(), Duration::from_secs(1));
}

Observed behavior:

The first call panics while adding a huge duration to SystemTime. Because this happens while the mutex guard is held, the mutex becomes poisoned. A later normal call then panics on self.0.lock().unwrap().

Expected behavior:

Extreme token lifetime values should return TokenReuseError or another controlled error. The token log should also avoid making one bad call poison future validations.

Example 2: ACK accounting can overflow in congestion controllers

NewReno:

if self.window < self.ssthresh {
    // Slow start
    self.window += bytes;
}

BBR:

self.max_bandwidth
    .on_ack(now, sent, bytes, self.round_count, app_limited);
self.acked_bytes += bytes;

and:

self.aggregation_epoch_bytes += newly_acked_bytes;
let diff = self.aggregation_epoch_bytes - expected_bytes_acked;

Minimal NewReno reproducer shape:

use std::{mem::MaybeUninit, sync::Arc, time::Duration};
use quinn_proto::RttEstimator;
use quinn_proto::congestion::{ControllerFactory, NewRenoConfig};

#[test]
#[should_panic]
fn new_reno_ack_overflow() {
    let now = std::time::Instant::now();
    let mut controller = Arc::new(NewRenoConfig::default()).build(now, 1200);

    let rtt = unsafe { MaybeUninit::<RttEstimator>::zeroed().assume_init() };

    controller.on_ack(
        now,
        now + Duration::from_nanos(1),
        u64::MAX,
        false,
        &rtt,
    );
}

Observed behavior:

In debug/test builds, the controller panics on integer overflow instead of saturating, rejecting, or otherwise containing the bad accounting value.

Severity note:

These tests call congestion-controller APIs directly and use constructed/extreme ACK byte counts. I would frame this as a robustness hardening issue, not as a standalone remote exploit claim.

Lower-priority generated cases

Custom crypto/session invariant cases

These are reproducible, but they depend on custom trait implementations returning inconsistent state:

  • init_0rtt panics if early_crypto() is present but transport_parameters() returns Ok(None):
let params = params
    .expect("crypto layer didn't supply transport parameters with ticket");
  • initial_close can overflow if a custom packet key returns tag_len() == usize::MAX.

  • upgrade_crypto can panic if a custom session reaches 1-RTT upgrade while next_1rtt_keys() returns None.

These are useful hardening notes for custom crypto integration, but weaker than the token/congestion arithmetic cases.

Invalid enum / direct misuse cases

Generated tests also hit panics by forging invalid enum values or violating internal packet-processing assumptions:

  • Dir is forged with unsafe { transmute(2u8) }, then used in stream-limit APIs. Safe Rust callers cannot construct this value normally.
  • HeaderKey::decrypt is called with pn_offset == 0, which underflows pn_offset - 1.
  • poll_transmit debug assertions can be tripped by malformed output-buffer state.

I would not lead with these unless a safe public reproducer is found.

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