From 24b8d3ab7ab288b976d83e2a88dd97315fb9ca51 Mon Sep 17 00:00:00 2001 From: Pascal Seitz Date: Fri, 9 Jun 2023 20:30:02 +0800 Subject: [PATCH] invert checked-decode feature-flag to unchecked-decode invert checked-decode to unchecked-decode Previously setting default-features=false removed the bounds checks from checked-decode. This change inverts this, so it will need to be deliberatly need to be deactivated. --- .github/workflows/coverage.yml | 2 +- .github/workflows/rust.yml | 2 +- Cargo.toml | 2 +- README.md | 4 ++-- fuzz/Cargo.toml | 3 +-- miri_tests/Cargo.toml | 2 +- src/block/decompress.rs | 22 +++++++++++----------- src/block/decompress_safe.rs | 4 ++-- src/lib.rs | 5 +++-- tests/tests.rs | 10 +++------- 10 files changed, 26 insertions(+), 30 deletions(-) diff --git a/.github/workflows/coverage.yml b/.github/workflows/coverage.yml index bc00e5c0..54647b10 100644 --- a/.github/workflows/coverage.yml +++ b/.github/workflows/coverage.yml @@ -21,7 +21,7 @@ jobs: - name: Generate code coverage all-features run: cargo llvm-cov --all-features --workspace --lcov --output-path lcov2.info - name: Generate code coverage unsafe w. checked-decode - run: cargo llvm-cov --no-default-features --features frame --features checked-decode --workspace --lcov --output-path lcov3.info + run: cargo llvm-cov --no-default-features --features frame --workspace --lcov --output-path lcov3.info - name: Upload coverage to Codecov uses: codecov/codecov-action@v1 with: diff --git a/.github/workflows/rust.yml b/.github/workflows/rust.yml index 3fe79c03..ec10f56c 100644 --- a/.github/workflows/rust.yml +++ b/.github/workflows/rust.yml @@ -40,7 +40,7 @@ jobs: - name: Run tests --no-default-features with frame, with nightly features run: cargo +nightly test --no-default-features --features frame --features nightly - name: Run tests unsafe with checked-decode and frame - run: cargo test --no-default-features --features checked-decode --features frame + run: cargo test --no-default-features --features frame - name: Install cargo fuzz run: cargo install cargo-fuzz - name: Run fuzz tests (safe) diff --git a/Cargo.toml b/Cargo.toml index 0ff1b95d..7e2fc25f 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -44,7 +44,7 @@ git = "https://github.com/main--/rust-lz-fear" default = ["std", "safe-encode", "safe-decode", "frame"] safe-decode = [] safe-encode = [] -checked-decode = [] +unchecked-decode = [] # Removes some checks for additional performance. Only enable on trusted input! frame = ["std", "dep:twox-hash"] std = [] # use nightly compiler features diff --git a/README.md b/README.md index 85818d26..0ed4b1d6 100644 --- a/README.md +++ b/README.md @@ -54,9 +54,9 @@ Performance: lz4_flex = { version = "0.10", default-features = false } ``` -Warning: If you don't trust your input and your are using the Block format, use checked-decode in order to avoid out of bounds access. When using the Frame format make sure to enable checksums. +If you know your data is valid and not broken or corrupted, you can use `unchecked-decode` feature-flag to get a little bit more performance. ``` -lz4_flex = { version = "0.10", default-features = false, features = ["checked-decode"] } +lz4_flex = { version = "0.10", default-features = false, features = ["unchecked-decode"] } ``` ```rust diff --git a/fuzz/Cargo.toml b/fuzz/Cargo.toml index 71ad5ed4..547c7496 100644 --- a/fuzz/Cargo.toml +++ b/fuzz/Cargo.toml @@ -19,8 +19,7 @@ arbitrary = { version = "1", features = ["derive"] } libfuzzer-sys = "0.4" lzzzz = "0.8" -# checked-decode" is required for the fuzz_decomp_corrupted_data target -lz4_flex = { path = "..", default-features = false, features=["frame", "checked-decode"] } +lz4_flex = { path = "..", default-features = false, features=["frame"] } # Prevent this from interfering with workspaces [workspace] diff --git a/miri_tests/Cargo.toml b/miri_tests/Cargo.toml index b5edf646..7bf820c8 100644 --- a/miri_tests/Cargo.toml +++ b/miri_tests/Cargo.toml @@ -7,5 +7,5 @@ edition = "2021" [dependencies] rand = "0.8.5" -lz4_flex = { path = "..", default-features = false, features=["frame", "checked-decode"] } +lz4_flex = { path = "..", default-features = false, features=["frame"] } diff --git a/src/block/decompress.rs b/src/block/decompress.rs index fbeba5ce..3f1e4391 100644 --- a/src/block/decompress.rs +++ b/src/block/decompress.rs @@ -96,7 +96,7 @@ unsafe fn copy_from_dict( // If we're here we know offset > output pos, so we have at least 1 byte to copy from dict debug_assert!(output_ptr.offset_from(output_base) >= 0); debug_assert!(offset > output_ptr.offset_from(output_base) as usize); - // If checked-decode is enabled we also know that the offset falls within ext_dict + // If unchecked-decode is not disabled we also know that the offset falls within ext_dict debug_assert!(ext_dict.len() + output_ptr.offset_from(output_base) as usize >= offset); let dict_offset = ext_dict.len() + output_ptr.offset_from(output_base) as usize - offset; @@ -138,7 +138,7 @@ fn read_integer_ptr( loop { // We add the next byte until we get a byte which we add to the counting variable. - #[cfg(feature = "checked-decode")] + #[cfg(not(feature = "unchecked-decode"))] { if *input_ptr >= _input_ptr_end { return Err(DecompressError::ExpectedAnotherByte); @@ -283,7 +283,7 @@ pub(crate) fn decompress_internal( let offset = read_u16_ptr(&mut input_ptr) as usize; let output_len = unsafe { output_ptr.offset_from(output_base) as usize }; - #[cfg(feature = "checked-decode")] + #[cfg(not(feature = "unchecked-decode"))] { if offset > output_len + ext_dict.len() { return Err(DecompressError::OffsetOutOfBounds); @@ -341,7 +341,7 @@ pub(crate) fn decompress_internal( literal_length += read_integer_ptr(&mut input_ptr, input_ptr_end)? as usize; } - #[cfg(feature = "checked-decode")] + #[cfg(not(feature = "unchecked-decode"))] { // Check if literal is out of bounds for the input, and if there is enough space on // the output @@ -370,7 +370,7 @@ pub(crate) fn decompress_internal( } // Read duplicate section - #[cfg(feature = "checked-decode")] + #[cfg(not(feature = "unchecked-decode"))] { if (input_ptr_end as usize) - (input_ptr as usize) < 2 { return Err(DecompressError::ExpectedAnotherByte); @@ -396,8 +396,8 @@ pub(crate) fn decompress_internal( // by simply referencing the other location. let output_len = unsafe { output_ptr.offset_from(output_base) as usize }; - // We'll do a bounds check in checked-decode. - #[cfg(feature = "checked-decode")] + // We'll do a bounds check except unchecked-decode is enabled. + #[cfg(not(feature = "unchecked-decode"))] { if offset > output_len + ext_dict.len() { return Err(DecompressError::OffsetOutOfBounds); @@ -415,7 +415,7 @@ pub(crate) fn decompress_internal( copy_from_dict(output_base, &mut output_ptr, ext_dict, offset, match_length) }; if copied == match_length { - #[cfg(feature = "checked-decode")] + #[cfg(not(feature = "unchecked-decode"))] { if input_ptr >= input_ptr_end { return Err(DecompressError::ExpectedAnotherByte); @@ -438,7 +438,7 @@ pub(crate) fn decompress_internal( unsafe { duplicate(&mut output_ptr, output_end, start_ptr, match_length); } - #[cfg(feature = "checked-decode")] + #[cfg(not(feature = "unchecked-decode"))] { if input_ptr >= input_ptr_end { return Err(DecompressError::ExpectedAnotherByte); @@ -539,8 +539,8 @@ mod test { assert_eq!(decompress(&[0x30, b'a', b'4', b'9'], 3).unwrap(), b"a49"); } - // this error test is only valid in checked-decode. - #[cfg(feature = "checked-decode")] + // this error test is only valid with checked-decode. + #[cfg(not(feature = "unchecked-decode"))] #[test] fn offset_oob() { decompress(&[0x10, b'a', 2, 0], 4).unwrap_err(); diff --git a/src/block/decompress_safe.rs b/src/block/decompress_safe.rs index eb848d45..860f7b7c 100644 --- a/src/block/decompress_safe.rs +++ b/src/block/decompress_safe.rs @@ -184,7 +184,7 @@ pub(crate) fn decompress_internal( if literal_length > input.len() - input_pos { return Err(DecompressError::LiteralOutOfBounds); } - #[cfg(feature = "checked-decode")] + #[cfg(not(feature = "unchecked-decode"))] if literal_length > output.capacity() - output.pos() { return Err(DecompressError::OutputTooSmall { expected: output.pos() + literal_length, @@ -217,7 +217,7 @@ pub(crate) fn decompress_internal( match_length += read_integer(input, &mut input_pos)? as usize; } - #[cfg(feature = "checked-decode")] + #[cfg(not(feature = "unchecked-decode"))] if output.pos() + match_length > output.capacity() { return Err(DecompressError::OutputTooSmall { expected: output.pos() + match_length, diff --git a/src/lib.rs b/src/lib.rs index efeff5ea..6f2a4de3 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -60,8 +60,9 @@ //! //! - `safe-encode` uses only safe rust for encode. _enabled by default_ //! - `safe-decode` uses only safe rust for encode. _enabled by default_ -//! - `checked-decode` will add additional checks if `safe-decode` is not enabled, to avoid out of -//! bounds access. This should be enabled for untrusted input. +//! - `unchecked-decode` will remove checks to avoid out of +//! bounds reads on corrupted data. This should be only enabled for trusted input (e.g. +//! checksummed). //! - `frame` support for LZ4 frame format. _implies `std`, enabled by default_ //! - `std` enables dependency on the standard library. _enabled by default_ //! diff --git a/tests/tests.rs b/tests/tests.rs index 8d5df321..393a0f60 100644 --- a/tests/tests.rs +++ b/tests/tests.rs @@ -320,22 +320,20 @@ fn print_compression_ration(input: &'static [u8], name: &str) { // } #[cfg(test)] +#[cfg(not(feature = "unchecked-decode"))] mod checked_decode { use super::*; - #[cfg_attr(not(feature = "checked-decode"), ignore)] #[test] fn error_case_1() { let _err = decompress_size_prepended(&[122, 1, 0, 1, 0, 10, 1, 0]); } - #[cfg_attr(not(feature = "checked-decode"), ignore)] #[test] fn error_case_2() { let _err = decompress_size_prepended(&[ 44, 251, 49, 0, 0, 0, 8, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 16, 0, 0, 0, 0, 0, 0, 0, 0, ]); } - #[cfg_attr(not(feature = "checked-decode"), ignore)] #[test] fn error_case_3() { let _err = decompress_size_prepended(&[ @@ -343,13 +341,11 @@ mod checked_decode { ]); } - #[cfg_attr(not(feature = "checked-decode"), ignore)] #[test] fn error_case_4() { let _err = decompress_size_prepended(&[0, 61, 0, 0, 0, 7, 0]); } - #[cfg_attr(not(feature = "checked-decode"), ignore)] #[test] fn error_case_5() { let _err = decompress_size_prepended(&[8, 0, 0, 0, 4, 0, 0, 0]); @@ -514,7 +510,7 @@ fn test_decomp(data: &[u8]) { fn bug_fuzz_7() { #[cfg(not(feature = "safe-decode"))] { - #[cfg(not(feature = "checked-decode"))] + #[cfg(feature = "unchecked-decode")] { return; } @@ -528,7 +524,7 @@ fn bug_fuzz_7() { } // TODO maybe also not panic for default feature flags -#[cfg(all(not(feature = "safe-decode"), feature = "checked-decode"))] +#[cfg(all(not(feature = "safe-decode"), feature = "unchecked-decode"))] #[test] fn bug_fuzz_8() { let data = &[