UB in `inflate::State::extra()`

This is reported by Miri under tests gzip_chunked_n_bytes(), where n is some chunk length that is less than extra_max. On release v0.3.0.

error: Undefined Behavior: out-of-bounds pointer arithmetic: expected a pointer to 68 bytes of memory, but got alloc1666089 which is only 64 bytes from the end of the allocation
    --> /home/inahga/Projects/zlib-rs/zlib-rs/src/inflate.rs:698:33
     |
698  | ...                   head.extra.add(written_so_far),
     |                       ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ out-of-bounds pointer arithmetic: expected a pointer to 68 bytes of memory, but got alloc1666089 which is only 64 bytes from the end of the allocation
     |
     = help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
     = help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
help: alloc1666089 was allocated here:
    --> test-libz-rs-sys/src/inflate.rs:1341:13
     |
1341 |         let mut extra_buf = [0u8; 64];
     |             ^^^^^^^^^^^^^
     = note: BACKTRACE (of the first span) on thread `inflate::gzip_c`:
     = note: inside `zlib_rs::inflate::State::<'_>::extra` at /home/inahga/Projects/zlib-rs/zlib-rs/src/inflate.rs:698:33: 698:63
note: inside `zlib_rs::inflate::State::<'_>::dispatch`
    --> /home/inahga/Projects/zlib-rs/zlib-rs/src/inflate.rs:490:28
     |
490  |             Mode::Extra => self.extra(),
     |                            ^^^^^^^^^^^^
note: inside `zlib_rs::inflate::inflate`
    --> /home/inahga/Projects/zlib-rs/zlib-rs/src/inflate.rs:1910:19
     |
1910 |     let mut err = state.dispatch();
     |                   ^^^^^^^^^^^^^^^^
note: inside `libz_rs_sys::inflate`
    --> /home/inahga/Projects/zlib-rs/libz-rs-sys/src/lib.rs:321:9
     |
321  |         zlib_rs::inflate::inflate(stream, flush) as _
     |         ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `inflate::gzip_chunked`
    --> test-libz-rs-sys/src/inflate.rs:1368:32
     |
1368 | ...afe { inflate(stream, InflateFlush::NoFlush as _) };
     |          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
note: inside `inflate::gzip_chunked_16_bytes`
    --> test-libz-rs-sys/src/inflate.rs:1225:5
     |
1225 |     gzip_chunked(16);
     |     ^^^^^^^^^^^^^^^^
note: inside closure
    --> test-libz-rs-sys/src/inflate.rs:1224:27
     |
1223 | #[test]
     | ------- in this procedural macro expansion
1224 | fn gzip_chunked_16_bytes() {
     |                           ^
     = note: this error originates in the attribute macro `test` (in Nightly builds, run with -Z macro-backtrace for more info)

The culprit code is:

                    if !head.extra.is_null() {
                        let written_so_far = head.extra_len as usize - self.length;

                        let count = Ord::min(
                            (head.extra_max as usize).saturating_sub(written_so_far),
                            extra_slice.len(),
                        );

                        unsafe {
                            core::ptr::copy_nonoverlapping(
                                self.bit_reader.as_ptr(),
                                head.extra.add(written_so_far),
                                count,
                            );
                        }
                    }

When we approach the end of extra_max with too large of an extra, written_so_far will evaluate to a non-zero value, while count will appropriately evaluate to zero. But, since we're at the end of what the buffer, we attempt to head.extra.add() past the allocated area.

We don't actually write anything (fortunately), since count is zero, but according to the docs for ptr::add(), it is UB to move the pointer beyond its allocated space:

    /// * If the computed offset is non-zero, then `self` must be derived from a pointer to some
    ///   [allocated object], and the entire memory range between `self` and the result must be in
    ///   bounds of that allocated object. In particular, this range must not "wrap around" the edge
    ///   of the address space.

I'm able to straightforwardly fix this by wrapping copy_nonoverlapping() in a if count > 0 block. However, I see other fields like comment and name parse out this field in a different way which looks cleaner and does not have this problem, so it may be better to refactor extra to look like those.

trifectatechfoundation / zlib-rs

UB in `inflate::State::extra()` #195