search

PSeitz / lz4_flex

Fastest pure Rust implementation of LZ4 compression/decompression.
MIT License
441 stars 28 forks source link

Undefined Behavior with dict and pointer arithmetic #49

Closed PSeitz closed 2 years ago

PSeitz commented 2 years ago

The add method on https://doc.rust-lang.org/std/primitive.pointer.html#safety-5 mentions that:

"Both the starting and resulting pointer must be either in bounds or one byte past the end of the same allocated object."

But there are several places, where pointer arithmetic is used to move outside an allocated object.

MIRIFLAGS="-Zmiri-disable-isolation -Zmiri-disable-stacked-borrows" cargo +nightly miri test --no-default-features fails for dict tests currently

@arthurprs

arthurprs commented 2 years ago

Oh wow, rust unsafe rules are really depressing :sweat_smile: I can try and cook a fix soon.

It sort of makes sense. I don't think it's possible in practice but the pointer could underflow and break the comparisons. I'm almost sure the C implementations is riddled with those kinds of assumptions.

PSeitz commented 2 years ago

hehe they are quite strict, but I think they are just better in documenting it. I read the same applies for C (in a Rust thread though).

There are several places where ptr.add to check out of bounds. I think they can to be replaced with a cast to usize or wrapping_add, which is oddly not unsafe until dereferencing.

I also saw a suspicious comment: // match crosses ext_dict and output