Skip to content

Unsigned underflow yields panicable constraint sizes #182

@this-vishalsingh

Description

@this-vishalsingh

Context: crates/whir/src/lib.rs

Description

The SparseStatement does not enforce the invariant total_num_variables >= point.len(). As a result, selector_num_variables() performs unchecked usize subtraction and can underflow (wrapping to a huge value in release builds).
Downstream code uses selector_num_variables() for indexing, slicing, and bit-shifts (e.g., 1 << selector_num_variables() and iterating 0..selector_num_variables()), which can panic due to out-of-bounds slicing or oversized shifts.

A real exploitation path is a verifier/prover service that accepts attacker-controlled SparseStatement: the attacker supplies a statement with total_num_variables smaller than point.len(), triggering a panic during verification/proving and causing a denial of service.

impl<EF> SparseStatement<EF> {
    pub fn new(total_num_variables: usize, point: MultilinearPoint<EF>, values: Vec<SparseValue<EF>>) -> Self {
        Self {
            total_num_variables,
            point,
            values,
        }
    }

    pub fn selector_num_variables(&self) -> usize {
        self.total_num_variables - self.inner_num_variables()
    }

    pub fn inner_num_variables(&self) -> usize {
        self.point.len()
    }
}

Recommendation

  • Enforce total_num_variables >= point.len() at construction time (e.g., make constructors return Result and validate), and compute selector_num_variables with checked_sub (returning an error on invalid statements).
  • Also prefer returning structured errors instead of panicking in verification/proving paths.

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