Uses of slice::from_raw_parts with input from FFI would need some manual pre-checks

[glandium]

The preconditions for slice::from_raw_parts contain the following:

data must be non-null and aligned even for zero-length slices. One reason for this is that enum layout optimizations may rely on references (including slices of any length) being aligned and non-null to distinguish them from other data. You can obtain a pointer that is usable as data for zero-length slices using NonNull::dangling().

When the preconditions are not fulfilled, a debug build will fail with the message

unsafe precondition(s) violated: slice::from_raw_parts requires the pointer to be aligned and non-null, and the total size of the slice not to exceed isize::MAX) at library\core\src\panicking.rs:220

and a release build may crash depending on what kind of optimizations the rust compiler allowed itself.

One way such thing can happen is when using the crc32 function like the following:

crc32(0, NULL, 0)

All the uses of slice::from_raw_parts should be audited as to whether the pointer/length pairs can originate from API user input in a similar way, and adjusted to make the preconditions met.

[folkertdev]

https://github.com/memorysafety/zlib-rs/pull/145 fixes the immediate issue with crc32/adler32 and NULL. The NULL input is actually useful there because it gives the initial checksum value.

I'm working on other public-facing calls to unsafe functions (core::ptr::read and the from_raw_parts calls) and adding # Safety docs for the remaining functions.

In most cases the resonsibility of satisfying the preconditions just gets passed on to the caller, but for NULL in particular there is often (but not always) a special error that gets returned. So this requires some care to make sure we actually mirror the error output, and I plan to also return errors when stock zlib segfaults on NULL pointers.

[folkertdev]

On main, the public C api functions now all have a # Safety section that documents the assumptions the function makes. We also now have 100% branch coverage in that file, so edge cases (like NULL pointers) should be checked.

We return Z_STREAM_ERROR in some cases where stock zlib segfaults. Segfaults indicate UB so I believe that us returning a value is within the contract of the function.

Of course, these changes benefit from a second pair of eyes going over the assumptions.

Then there is of course also unsafe code internal to the library, but that already got tested (with miri too)/fuzzed a lot more.