AsciiSet::contains() causes out-of-bounds array access for non-ASCII bytes (byte >= 128)

[yanfenghu]

Hi,

I found a potential memory safety issue in the AsciiSet implementation through Kani formal verification.

Location: src/ascii_set.rs:47, 58, 64

Current Code:

const ASCII_RANGE_LEN: usize = 0x80; // 128
const BITS_PER_CHUNK: usize = 32;

pub struct AsciiSet {
mask: [Chunk; ASCII_RANGE_LEN / BITS_PER_CHUNK], // [u32; 4]
}

impl AsciiSet {
pub(crate) const fn contains(&self, byte: u8) -> bool {
let chunk = self.mask[byte as usize / BITS_PER_CHUNK]; // OOB when byte >= 128!
// …
}

pub const fn add(&self, byte: u8) -> Self {
mask[byte as usize / BITS_PER_CHUNK] |= ... // OOB when byte >= 128!
// ...
}

pub const fn remove(&self, byte: u8) -> Self {
mask[byte as usize / BITS_PER_CHUNK] &= ... // OOB when byte >= 128!
// ...
}
}
Analysis:

Array mask has 4 elements (valid indices: 0-3)

Index calculation: byte as usize / 32

For byte >= 128: index >= 4 (out of bounds)

Current Mitigation:
The documentation mentions this is for ASCII only, and should_percent_encode() checks is_ascii() before calling. However, add() and remove() are public API.

Impact:

Potential out-of-bounds memory access

Undefined behavior

Security concern if untrusted input reaches these methods

Suggested Fix:

pub(crate) const fn contains(&self, byte: u8) -> bool {
if byte >= 0x80 { return false; }
let chunk = self.mask[byte as usize / BITS_PER_CHUNK];
let mask = 1 << (byte as usize % BITS_PER_CHUNK);
(chunk & mask) != 0
}

pub const fn add(&self, byte: u8) -> Self {
let mut mask = self.mask;
if byte < 0x80 {
mask[byte as usize / BITS_PER_CHUNK] |= 1 << (byte as usize % BITS_PER_CHUNK);
}
Self { mask }
}
Could you please confirm if this is a valid security concern?

[emilio]

Doesn't look like it. It's a safe panic right?

[yanfenghu]

You are absolutely right. In Rust, an out-of-bounds array access triggers a panic rather than memory corruption or Undefined Behavior, so it is “safe” from a strict memory-safety perspective.

However, my main concern is robustness and Denial of Service (DoS). Since add() and remove() are part of the public API, passing a non-ASCII byte (e.g., from untrusted input without prior validation) will cause the application to crash unexpectedly.

While panicking on invalid input is sometimes acceptable, defensive programming suggests that a setter/modifier like add() or remove() shouldn’t crash the whole program when encountering a value outside the ASCII range (0…=127), especially when a simple and zero-cost check (if byte < 0x80) can gracefully ignore it or handle it safely.

Would you be open to adding the boundary check to make the public API more robust against unexpected panics, or do you prefer to strictly rely on the caller to pre-validate the input?