Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

invert checked-decode to unchecked-decode #134

Merged
merged 1 commit into from Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/coverage.yml
Expand Up @@ -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:
Expand Down
2 changes: 1 addition & 1 deletion .github/workflows/rust.yml
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion Cargo.toml
Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions README.md
Expand Up @@ -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
Expand Down
3 changes: 1 addition & 2 deletions fuzz/Cargo.toml
Expand Up @@ -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]
Expand Down
2 changes: 1 addition & 1 deletion miri_tests/Cargo.toml
Expand Up @@ -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"] }

22 changes: 11 additions & 11 deletions src/block/decompress.rs
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -283,7 +283,7 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
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);
Expand Down Expand Up @@ -341,7 +341,7 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
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
Expand Down Expand Up @@ -370,7 +370,7 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
}

// 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);
Expand All @@ -396,8 +396,8 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
// 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);
Expand All @@ -415,7 +415,7 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
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);
Expand All @@ -438,7 +438,7 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
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);
Expand Down Expand Up @@ -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();
Expand Down
4 changes: 2 additions & 2 deletions src/block/decompress_safe.rs
Expand Up @@ -184,7 +184,7 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
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,
Expand Down Expand Up @@ -217,7 +217,7 @@ pub(crate) fn decompress_internal<const USE_DICT: bool, S: Sink>(
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,
Expand Down
5 changes: 3 additions & 2 deletions src/lib.rs
Expand Up @@ -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_
//!
Expand Down
10 changes: 3 additions & 7 deletions tests/tests.rs
Expand Up @@ -320,36 +320,32 @@ 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(&[
7, 0, 0, 0, 0, 0, 0, 11, 0, 0, 7, 16, 0, 0, 0, 0, 0, 0, 0, 0, 0, 4, 1, 0, 0,
]);
}

#[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]);
Expand Down Expand Up @@ -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;
}
Expand All @@ -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 = &[
Expand Down