snarkVM icon indicating copy to clipboard operation
snarkVM copied to clipboard

[Bug] Controllable allocation size allows for memory leaks or panics (depending on size)

Open feezybabee opened this issue 2 years ago • 1 comments

https://hackerone.com/reports/2255800

Summary:

The snarkVM source code contains the Vec::with_capacity(capacity) pattern in many places, where capacity is a controllable non-sanitised value. The following places in code are good examples:

Such code can cause panic in rust, as well as allocating an excessive amount of memory. According to the description of the Vec::with_capacity() function:

Panics if the new capacity exceeds `isize::MAX` bytes.

Let's consider the following test case as an example:

use snarkvm_curves::bls12_377::Fr;
use snarkvm_fields::PoseidonGrainLFSR;

fn main() {
    let num_elements = 0x400000000000000;
    let mut lfsr = PoseidonGrainLFSR::new(false, 253, 3, 8, 32);
    let _res = lfsr.get_field_elements_rejection_sampling::<Fr>(num_elements);
}

This code will panic because:

  • the allocation size will be calculated as mem::size_of::<Fr>() * num_elements = 32 * 0x400000000000000 = 0x8000000000000000 (equal to isize::MAX + 1)
  • it will lead to panic caused by Vec::with_capacity() function

Proof-of-Concept (PoC)

Cargo.toml

[package]
name = "test"
version = "0.1.0"
edition = "2021"

# See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html

[dependencies]
snarkvm-curves = { path = "../snarkVM/curves" }
snarkvm-fields = { path = "../snarkVM/fields" }

src/main.rs

use snarkvm_curves::bls12_377::Fr;
use snarkvm_fields::PoseidonGrainLFSR;

fn main() {
    let num_elements = 0x400000000000000;
    let mut lfsr = PoseidonGrainLFSR::new(false, 253, 3, 8, 32);
    let _res = lfsr.get_field_elements_rejection_sampling::<Fr>(num_elements);
}

Result

$ RUST_BACKTRACE=1 ./target/release/test
thread 'main' panicked at 'capacity overflow', library/alloc/src/raw_vec.rs:524:5
stack backtrace:
   0: rust_begin_unwind
             at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/std/src/panicking.rs:593:5
   1: core::panicking::panic_fmt
             at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/core/src/panicking.rs:67:14
   2: alloc::raw_vec::capacity_overflow
             at /rustc/d5c2e9c342b358556da91d61ed4133f6f50fc0c3/library/alloc/src/raw_vec.rs:524:5
   3: snarkvm_fields::traits::poseidon_grain_lfsr::PoseidonGrainLFSR::get_field_elements_rejection_sampling
   4: test::main
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.

Impact

This problem may result in a remote DoS.

feezybabee avatar Jan 10 '24 02:01 feezybabee

I don't see either of the listed methods used with user-supplied input (just const values):

fields/src/traits/poseidon_default.rs
60:            ark.push(lfsr.get_field_elements_rejection_sampling(RATE + 1)?);
66:            let _ = lfsr.get_field_elements_mod_p::<F>(2 * (RATE + 1))?;
75:            let xs = lfsr.get_field_elements_mod_p::<F>(RATE + 1)?;
76:            let ys = lfsr.get_field_elements_mod_p::<F>(RATE + 1)?;

So this may not be possible to trigger.

ljedrz avatar Jan 10 '24 10:01 ljedrz

Closing with https://github.com/AleoNet/snarkVM/issues/2291#issuecomment-1884542664.

@feezybabee feel free to reopen if the original author has any additional information.

raychu86 avatar Jul 01 '24 17:07 raychu86