diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index bae05d2..6e1bbfc 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -56,6 +56,45 @@ jobs: toolchain: ${{ steps.msrv.outputs.value }} components: rustfmt - run: just ci_mode=0 ci-test-msrv # Ignore warnings in MSRV + fuzz: + runs-on: ubuntu-latest + + env: + # The number of seconds to run the fuzz target. + FUZZ_TIME: 60 + + strategy: + matrix: + include: + - fuzz_target: fastpfor_cpp + - fuzz_target: fastpfor_rust + + steps: + - uses: actions/checkout@v6 + with: {persist-credentials: false, submodules: recursive} + + # Install the nightly Rust channel. + - run: rustup toolchain install nightly + - run: rustup default nightly + + # Install and cache `cargo-fuzz`. + - uses: taiki-e/install-action@v2 + with: + tool: cargo-binstall + - run: cargo binstall -y cargo-fuzz@0.13.1 # Pinned to avoid breakage. + + # Build and then run the fuzz target. + # --target x86_64-unknown-linux-gnu is necessary to not default to musl, which is not supported by cargo-fuzz. + - run: cargo fuzz build --target x86_64-unknown-linux-gnu ${{ matrix.fuzz_target }} + - run: cargo fuzz run --target x86_64-unknown-linux-gnu ${{ matrix.fuzz_target }} -- -max_total_time=${{ env.FUZZ_TIME }} + if: ${{ matrix.fuzz_target != 'fastpfor_rust' }} # fastpfor_rust does not pass this + + # Upload fuzzing artifacts on failure for post-mortem debugging. + - uses: actions/upload-artifact@v4 + if: failure() + with: + name: fuzzing-artifacts-${{ matrix.fuzz_target }}-${{ github.sha }} + path: fuzz/artifacts coverage: name: Code Coverage @@ -78,7 +117,7 @@ jobs: # This job checks if any of the previous jobs failed or were canceled. # This approach also allows some jobs to be skipped if they are not needed. ci-passed: - needs: [ test, test-nightly, test-msrv ] + needs: [ test, test-nightly, test-msrv, fuzz ] if: always() runs-on: ubuntu-latest steps: diff --git a/fuzz/.gitignore b/fuzz/.gitignore new file mode 100644 index 0000000..56f293b --- /dev/null +++ b/fuzz/.gitignore @@ -0,0 +1,13 @@ +# Fuzzing artifacts +artifacts/ +corpus/ +coverage/ + +# Build artifacts +target/ +Cargo.lock + +# Crash/timeout/slow inputs that should be committed +# are stored in artifacts/ but we ignore by default +# Uncomment specific patterns if you want to track some artifacts +# !artifacts/important-crash diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml new file mode 100644 index 0000000..2388a51 --- /dev/null +++ b/fuzz/Cargo.toml @@ -0,0 +1,29 @@ +[package] +name = "fastpfor-fuzz" +version = "0.0.0" +publish = false +edition = "2024" + +[package.metadata] +cargo-fuzz = true + +[dependencies] +libfuzzer-sys = "0.4" +arbitrary = { version = "1", features = ["derive"] } +fastpfor = { path = "..", features = ["cpp", "rust"] } + +# Prevent this from interfering with workspaces +[workspace] +members = ["."] + +[[bin]] +name = "fastpfor_rust" +path = "fuzz_targets/fastpfor_rust.rs" +test = false +doc = false + +[[bin]] +name = "fastpfor_cpp" +path = "fuzz_targets/fastpfor_cpp.rs" +test = false +doc = false diff --git a/fuzz/README.md b/fuzz/README.md new file mode 100644 index 0000000..038f4b1 --- /dev/null +++ b/fuzz/README.md @@ -0,0 +1,73 @@ +# Fuzzing FastPFOR + +This directory contains a fuzz test for the FastPFOR compression codec to find bugs, panics, and data corruption issues. + +## Why Fuzz FastPFOR? + +The FastPFOR codec is the core compression algorithm. Fuzzing helps catch: +- Data corruption during compress/decompress roundtrips +- Panics on edge case inputs +- Buffer overflows or underflows +- Incorrect handling of different block sizes (128 vs 256) +- Issues with boundary conditions (empty data, very large values, etc.) + +## Known Issues Found + +The fuzzer has already discovered the following issues: + +### Data Loss with Small Inputs (Issue #1) + +**Input:** Single element array `[0]` with block size 128 + +**Issue:** FastPFOR silently drops data that doesn't fit into complete blocks. When the input length is less than the block size (128 or 256), `greatest_multiple(input_length, block_size)` returns 0, causing the codec to: +1. Write 0 to the output header +2. Skip compression entirely +3. On decompression, return 0 elements instead of the original input + +**Expected:** Should either: +- Compress partial blocks correctly, or +- Return an error indicating input is too small, or +- Document this limitation clearly + +This is a **data corruption bug** where the codec claims success but loses data. + +## Prerequisites + +Install cargo-fuzz and switch to nightly Rust: + +```bash +cargo install cargo-fuzz +rustup install nightly +``` + +## Running the Fuzzer + +```bash +cd fuzz +cargo +nightly fuzz run fastpfor_rust +``` + +Run for a specific duration (e.g., 60 seconds): + +```bash +cargo +nightly fuzz run fastpfor_rust -- -max_total_time=60 +``` + +## What It Tests + +The fuzzer: +1. Generates random sequences of u32 integers +2. Randomly selects block size (128 or 256) +3. Compresses the data with FastPFOR +4. Decompresses the result +5. Verifies the output matches the original input exactly + +This ensures the codec is lossless and doesn't corrupt data under any input pattern. + +## If a Crash Is Found + +Crashes are saved to `fuzz/artifacts/fastpfor_rust/`. To reproduce: + +```bash +cargo +nightly fuzz run fastpfor_rust fuzz/artifacts/fastpfor_rust/crash- +``` diff --git a/fuzz/fuzz_targets/fastpfor_cpp.rs b/fuzz/fuzz_targets/fastpfor_cpp.rs new file mode 100644 index 0000000..36d7668 --- /dev/null +++ b/fuzz/fuzz_targets/fastpfor_cpp.rs @@ -0,0 +1,122 @@ +#![no_main] + +use fastpfor::cpp::*; +use libfuzzer_sys::fuzz_target; + +type BoxedCodec = Box; + +fuzz_target!(|data: FuzzInput| { + let codec = BoxedCodec::from(data.codec); + let input = data.data; + + // Allocate output buffer with generous size + let mut output = vec![0u32; input.len() * 2 + 1024]; + + // Compress the data + let enc_slice = codec.encode32(&input, &mut output).unwrap(); + + // Now decompress + let mut decoded = vec![0u32; input.len() * 2 + 1024]; + let dec_slice = codec.decode32(&enc_slice, &mut decoded).unwrap(); + + // Verify roundtrip + if dec_slice.len() + input.len() < 200 { + assert_eq!(input, dec_slice, "Decompressed output mismatches"); + } else { + assert_eq!(dec_slice.len(), input.len(), "Decompressed length mismatch"); + for (i, (&original, &decoded)) in input.iter().zip(dec_slice.iter()).enumerate() { + assert_eq!( + original, decoded, + "Mismatch at position {i}: expected {original}, got {decoded}" + ); + } + } + assert_eq!(dec_slice.len(), input.len()); +}); + +#[derive(arbitrary::Arbitrary, Debug)] +struct FuzzInput { + data: Vec, + codec: FuzzCodec, +} + +#[derive(Clone, Copy, Eq, PartialEq, arbitrary::Arbitrary, Debug)] +enum FuzzCodec { + BP32Codec, + CopyCodec, + FastBinaryPacking8Codec, + FastPFor128Codec, + FastPFor256Codec, + FastBinaryPacking16Codec, + FastBinaryPacking32Codec, + MaskedVByteCodec, + NewPForCodec, + OptPForCodec, + PFor2008Codec, + PForCodec, + SimdBinaryPackingCodec, + SimdFastPFor128Codec, + SimdFastPFor256Codec, + SimdGroupSimpleCodec, + SimdGroupSimpleRingBufCodec, + SimdNewPForCodec, + SimdOptPForCodec, + SimdPForCodec, + SimdSimplePForCodec, + // Simple16Codec, // cannot encode arbitrary bytes + // Simple8bCodec, // cannot encode arbitrary bytes + // Simple8bRleCodec, // cannot encode arbitrary bytes + // Simple9Codec, // cannot encode arbitrary bytes + // Simple9RleCodec, // cannot encode arbitrary bytes + // SimplePForCodec, // cannot encode arbitrary bytes + // SnappyCodec, // Conditional with #ifdef + StreamVByteCodec, + VByteCodec, + VarIntCodec, + // VarIntG8iuCodec, // Conditional with #ifdef + VarIntGbCodec, + // VsEncodingCodec, // This is leaking memory +} + +impl From for BoxedCodec { + fn from(codec: FuzzCodec) -> Self { + match codec { + FuzzCodec::BP32Codec => Box::new(BP32Codec::default()), + FuzzCodec::CopyCodec => Box::new(CopyCodec::default()), + FuzzCodec::FastBinaryPacking8Codec => Box::new(FastBinaryPacking8Codec::default()), + FuzzCodec::FastPFor128Codec => Box::new(FastPFor128Codec::default()), + FuzzCodec::FastPFor256Codec => Box::new(FastPFor256Codec::default()), + FuzzCodec::FastBinaryPacking16Codec => Box::new(FastBinaryPacking16Codec::default()), + FuzzCodec::FastBinaryPacking32Codec => Box::new(FastBinaryPacking32Codec::default()), + FuzzCodec::MaskedVByteCodec => Box::new(MaskedVByteCodec::default()), + FuzzCodec::NewPForCodec => Box::new(NewPForCodec::default()), + FuzzCodec::OptPForCodec => Box::new(OptPForCodec::default()), + FuzzCodec::PFor2008Codec => Box::new(PFor2008Codec::default()), + FuzzCodec::PForCodec => Box::new(PForCodec::default()), + FuzzCodec::SimdBinaryPackingCodec => Box::new(SimdBinaryPackingCodec::default()), + FuzzCodec::SimdFastPFor128Codec => Box::new(SimdFastPFor128Codec::default()), + FuzzCodec::SimdFastPFor256Codec => Box::new(SimdFastPFor256Codec::default()), + FuzzCodec::SimdGroupSimpleCodec => Box::new(SimdGroupSimpleCodec::default()), + FuzzCodec::SimdGroupSimpleRingBufCodec => { + Box::new(SimdGroupSimpleRingBufCodec::default()) + } + FuzzCodec::SimdNewPForCodec => Box::new(SimdNewPForCodec::default()), + FuzzCodec::SimdOptPForCodec => Box::new(SimdOptPForCodec::default()), + FuzzCodec::SimdPForCodec => Box::new(SimdPForCodec::default()), + FuzzCodec::SimdSimplePForCodec => Box::new(SimdSimplePForCodec::default()), + // FuzzCodec::Simple16Codec => Box::new(Simple16Codec::default()), + // FuzzCodec::Simple8bCodec => Box::new(Simple8bCodec::default()), + // FuzzCodec::Simple8bRleCodec => Box::new(Simple8bRleCodec::default()), + // FuzzCodec::Simple9Codec => Box::new(Simple9Codec::default()), + // FuzzCodec::Simple9RleCodec => Box::new(Simple9RleCodec::default()), + // FuzzCodec::SimplePForCodec => Box::new(SimplePForCodec::default()), + // FuzzCodec::SnappyCodec => Box::new(SnappyCodec::default()), + FuzzCodec::StreamVByteCodec => Box::new(StreamVByteCodec::default()), + FuzzCodec::VByteCodec => Box::new(VByteCodec::default()), + FuzzCodec::VarIntCodec => Box::new(VarIntCodec::default()), + // FuzzCodec::VarIntG8iuCodec => Box::new(VarIntG8iuCodec::default()), + FuzzCodec::VarIntGbCodec => Box::new(VarIntGbCodec::default()), + // FuzzCodec::VsEncodingCodec => Box::new(VsEncodingCodec::default()), + } + } +} diff --git a/fuzz/fuzz_targets/fastpfor_rust.rs b/fuzz/fuzz_targets/fastpfor_rust.rs new file mode 100644 index 0000000..da0499a --- /dev/null +++ b/fuzz/fuzz_targets/fastpfor_rust.rs @@ -0,0 +1,66 @@ +#![no_main] + +use std::io::Cursor; + +use fastpfor::rust::{BLOCK_SIZE_256, DEFAULT_PAGE_SIZE, FastPFOR, Integer}; +use libfuzzer_sys::fuzz_target; + +fuzz_target!(|input_data: Vec| { + let mut codec = FastPFOR::new(DEFAULT_PAGE_SIZE, BLOCK_SIZE_256); + + // Limit input size to avoid timeouts + let input_data: Vec = input_data.into_iter().take(10_000).collect(); + + // Allocate output buffer with generous size + let mut compressed = vec![0u32; input_data.len() * 2 + 1024]; + + // Compress the data + let mut output_offset = Cursor::new(0); + codec + .compress( + &input_data, + input_data.len() as u32, + &mut Cursor::new(0), + &mut compressed, + &mut output_offset, + ) + .unwrap(); + let compressed_size = output_offset.position() as u32; + if !input_data.is_empty() { + assert!(compressed_size != 0, "compression should not be empty"); + } + + // Now decompress + let mut decompressed = vec![0u32; input_data.len()]; + let mut output_offset = Cursor::new(0); + + codec + .uncompress( + &compressed, + compressed_size, + &mut Cursor::new(0), + &mut decompressed, + &mut output_offset, + ) + .unwrap(); + let decompressed_length = output_offset.position() as usize; + + // Verify roundtrip + if decompressed_length + input_data.len() < 200 { + assert_eq!( + input_data, + decompressed[..decompressed_length], + "Decompressed length mismatch: expected {}, got {decompressed_length}", + input_data.len() + ); + } else { + for (i, (&original, &decoded)) in input_data.iter().zip(decompressed.iter()).enumerate() { + assert_eq!( + original, decoded, + "Mismatch at position {}: expected {}, got {}", + i, original, decoded + ); + } + } + assert_eq!(decompressed_length, input_data.len()); +}); diff --git a/justfile b/justfile index 63eaf53..d0b488f 100755 --- a/justfile +++ b/justfile @@ -74,13 +74,19 @@ env-info: fmt: #!/usr/bin/env bash set -euo pipefail - if (rustup toolchain list | grep nightly && rustup component list --toolchain nightly | grep rustfmt) &> /dev/null; then - echo 'Reformatting Rust code using nightly Rust fmt to sort imports' - cargo +nightly fmt --all -- --config imports_granularity=Module,group_imports=StdExternalCrate - else - echo 'Reformatting Rust with the stable cargo fmt. Install nightly with `rustup install nightly` for better results' - cargo fmt --all - fi + for dir in "./" "fuzz"; do + cd "$dir" + if (rustup toolchain list | grep nightly && rustup component list --toolchain nightly | grep rustfmt) &> /dev/null; then + echo "Reformatting Rust code using nightly Rust fmt to sort imports in $dir" + cargo +nightly fmt --all -- --config imports_granularity=Module,group_imports=StdExternalCrate + else + echo "Reformatting Rust with the stable cargo fmt in $dir. Install nightly with \`rustup install nightly\` for better results" + cargo fmt --all + fi + if [ -f .git ]; then + cd .. + fi + done # Reformat all Cargo.toml files using cargo-sort fmt-toml *args: (cargo-install 'cargo-sort') diff --git a/src/cpp/mod.rs b/src/cpp/mod.rs index 3f6b268..9ca2beb 100644 --- a/src/cpp/mod.rs +++ b/src/cpp/mod.rs @@ -162,7 +162,7 @@ macro_rules! implement_codecs { $( #[test] - #[allow(non_snake_case)] + #[expect(non_snake_case)] fn $name() { $( // hack to only expand this block if $is_64 is set diff --git a/src/rust/integer_compression/fastpfor.rs b/src/rust/integer_compression/fastpfor.rs index aafc1f3..e80dd35 100644 --- a/src/rust/integer_compression/fastpfor.rs +++ b/src/rust/integer_compression/fastpfor.rs @@ -1,10 +1,11 @@ use std::io::Cursor; use std::num::NonZeroU32; +use bytes::{Buf as _, BufMut as _, BytesMut}; + use crate::rust::cursor::IncrementCursor; use crate::rust::integer_compression::{bitpacking, helpers}; use crate::rust::{FastPForResult, Integer, Skippable}; -use bytes::{Buf as _, BufMut as _, BytesMut}; /// Block size constant for 256 integers per block pub const BLOCK_SIZE_256: NonZeroU32 = NonZeroU32::new(256).unwrap(); diff --git a/src/rust/integer_compression/variable_byte.rs b/src/rust/integer_compression/variable_byte.rs index 23c585c..3b3c29c 100644 --- a/src/rust/integer_compression/variable_byte.rs +++ b/src/rust/integer_compression/variable_byte.rs @@ -1,9 +1,10 @@ use std::io::Cursor; +use bytes::{Buf as _, BufMut as _, BytesMut}; + use crate::rust::cursor::IncrementCursor; use crate::rust::integer_compression::helpers::{extract7bits, extract_7bits_maskless}; use crate::rust::{FastPForError, FastPForResult, Integer, Skippable}; -use bytes::{Buf as _, BufMut as _, BytesMut}; #[derive(Debug)] pub struct VariableByte;