block-buffer: fix exception safety (#1487)

Users may trigger UB by observing broken type invariant using panicking
closures and `catch_unwind`.

This PR introduces guards which reset the buffer types when dealing with
user-provided closures.

Author: newpavlov

.github/workflows/block-buffer.yml

@@ -64,3 +64,19 @@ jobs:
           toolchain: ${{ matrix.rust }}
       - run: cargo test
       - run: cargo test --all-features
+
+  miri:
+    runs-on: ubuntu-latest
+    strategy:
+      matrix:
+        target:
+          - x86_64-unknown-linux-gnu
+          - s390x-unknown-linux-gnu
+    steps:
+      - uses: actions/checkout@v6
+      - uses: RustCrypto/actions/cargo-cache@master
+      - uses: dtolnay/rust-toolchain@master
+        with:
+          toolchain: nightly
+          components: miri
+      - run: cargo miri test --all-features --target ${{ matrix.target }}

block-buffer/CHANGELOG.md

@@ -4,6 +4,12 @@ All notable changes to this project will be documented in this file.
 The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.0.0/),
 and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.html).
 
+## 0.12.1 (UNRELEASED)
+### Fixed
+- Exception safety bug ([#1487])
+
+[#1487]: https://github.com/RustCrypto/utils/pull/1487
+
 ## 0.12.0 (2026-02-24)
 ### Added
 - `BlockSizes` trait implemented for sizes bigger than `U0` and smaller than `U256` ([#1455])

block-buffer/src/lib.rs

@@ -184,12 +184,18 @@ impl<BS: BlockSizes, K: BufferKind> BlockBuffer<BS, K> {
         if pos != 0 {
             let (left, right) = input.split_at(rem);
             input = right;
+
+            let g = ResetGuard(self);
+            let buf = &mut g.0.buffer;
             // SAFETY: length of `left` is equal to number of remaining bytes in `buffer`,
             // so we can copy data into it and process `buffer` as fully initialized block.
+            // Note that this code can temporarily break the eager buffer invariant,
+            // but we reset the buffer immediately after `compress` using `Drop` impl of
+            // `ResetGuard`, so this code is safe even if `compress` panics.
             let block = unsafe {
-                let buf_ptr = self.buffer.as_mut_ptr().cast::<u8>().add(pos);
+                let buf_ptr = buf.as_mut_ptr().cast::<u8>().add(pos);
                 ptr::copy_nonoverlapping(left.as_ptr(), buf_ptr, left.len());
-                self.buffer.assume_init_ref()
+                buf.assume_init_ref()
             };
             compress(slice::from_ref(block));
         }
@@ -363,22 +369,34 @@ impl<BS: BlockSizes> BlockBuffer<BS, Eager> {
         suffix: &[u8],
         mut compress: impl FnMut(&Array<u8, BS>),
     ) {
-        assert!(suffix.len() <= BS::USIZE, "suffix is too long");
         let pos = self.get_pos();
-        let mut buf = self.pad_with_zeros();
-        buf[pos] = delim;
-
-        let n = self.size() - suffix.len();
-        if self.size() - pos - 1 < suffix.len() {
-            compress(&buf);
+        let size = self.size();
+        // Number of bytes remaining in the buffer after `delim` is written to it
+        // This never underflows since for eager buffers `size` is always greater than `pos`.
+        let pad_len = size - pos - 1;
+
+        let suffix_dst_pos = size
+            .checked_sub(suffix.len())
+            .expect("suffix must be smaller than buffer block size");
+
+        let g = ResetGuard(self);
+        // SAFETY: we fully initialize the buffer. Note that we may temporarily break
+        // the buffer invariant, but we restore it using `ResetGuard`,
+        // which works even if `compress` panics.
+        let buf = unsafe {
+            let p: *mut u8 = g.0.buffer.as_mut_ptr().cast::<u8>().add(pos);
+            ptr::write(p, delim);
+            ptr::write_bytes(p.add(1), 0, pad_len);
+            g.0.buffer.assume_init_mut()
+        };
+
+        if pad_len < suffix.len() {
+            compress(buf);
             buf.fill(0);
-            buf[n..].copy_from_slice(suffix);
-            compress(&buf);
-        } else {
-            buf[n..].copy_from_slice(suffix);
-            compress(&buf);
         }
-        self.reset();
+
+        buf[suffix_dst_pos..].copy_from_slice(suffix);
+        compress(buf);
     }
 
     /// Pad message with 0x80, zeros and 64-bit message length using
@@ -422,3 +440,12 @@ impl<BS: BlockSizes, K: BufferKind> Drop for BlockBuffer<BS, K> {
 
 #[cfg(feature = "zeroize")]
 impl<BS: BlockSizes, K: BufferKind> ZeroizeOnDrop for BlockBuffer<BS, K> {}
+
+/// Resets the referenced buffer on drop.
+struct ResetGuard<'a, BS: BlockSizes, K: BufferKind>(&'a mut BlockBuffer<BS, K>);
+
+impl<BS: BlockSizes, K: BufferKind> Drop for ResetGuard<'_, BS, K> {
+    fn drop(&mut self) {
+        self.0.reset();
+    }
+}

block-buffer/src/read.rs

@@ -110,8 +110,16 @@ impl<BS: BlockSizes> ReadBuffer<BS> {
         }
         assert!(read_len < BS::USIZE);
 
-        gen_block(&mut self.buffer);
-        read_fn(&self.buffer[..read_len]);
+        let g = ResetGuard(self);
+        let buf = &mut g.0.buffer;
+
+        // Note that generated block is likely to break the `ReadBuffer` invariant.
+        // We restore it using `set_pos_unchecked` below and in case if one of the closures
+        // panic the buffer gets reset by the guard.
+        gen_block(buf);
+        read_fn(&buf[..read_len]);
+
+        core::mem::forget(g);
 
         // We checked that `read_len` satisfies the `set_pos_unchecked` safety contract
         unsafe { self.set_pos_unchecked(read_len) };
@@ -180,3 +188,12 @@ impl<BS: BlockSizes> Drop for ReadBuffer<BS> {
 
 #[cfg(feature = "zeroize")]
 impl<BS: BlockSizes> zeroize::ZeroizeOnDrop for ReadBuffer<BS> {}
+
+/// Resets the referenced buffer on drop.
+struct ResetGuard<'a, BS: BlockSizes>(&'a mut ReadBuffer<BS>);
+
+impl<BS: BlockSizes> Drop for ResetGuard<'_, BS> {
+    fn drop(&mut self) {
+        self.0.reset();
+    }
+}

block-buffer/tests/mod.rs

@@ -9,8 +9,11 @@ use block_buffer::{
 };
 use hex_literal::hex;
 
+use core::{array, panic::AssertUnwindSafe};
+use std::panic::catch_unwind;
+
 #[test]
-fn test_eager_digest_pad() {
+fn test_eager_digest() {
     let mut buf = EagerBuffer::<U4>::default();
     let inputs = [
         &b"01234567"[..],
@@ -45,7 +48,7 @@ fn test_eager_digest_pad() {
 }
 
 #[test]
-fn test_lazy_digest_pad() {
+fn test_lazy_digest() {
     let mut buf = LazyBuffer::<U4>::default();
     let inputs = [
         &b"01234567"[..],
@@ -78,6 +81,38 @@ fn test_lazy_digest_pad() {
     assert_eq!(buf.get_pos(), 0);
 }
 
+#[test]
+fn digest_pad_combinations() {
+    let delim = 0x80;
+    let data: [u8; 7] = array::from_fn(|i| u8::try_from(i).unwrap());
+    let suffix: [u8; 7] = array::from_fn(|i| u8::try_from(i + 0x10).unwrap());
+
+    for data_len in 0..data.len() {
+        for suffix_len in 0..suffix.len() {
+            let data = &data[..data_len];
+            let suffix = &suffix[..suffix_len];
+            let mut buf = EagerBuffer::<U8>::default();
+
+            buf.digest_blocks(data, |_| panic!("should not be called"));
+
+            let mut accum = Vec::with_capacity(2 * buf.size());
+            buf.digest_pad(delim, suffix, |block| accum.extend_from_slice(block));
+
+            assert!(accum.len() <= 2 * buf.size());
+            assert_eq!(buf.get_pos(), 0);
+
+            let (res_data, rem) = accum.split_at(data_len);
+            let (res_delim, rem) = rem.split_at(1);
+            let (res_zeros, res_suffix) = rem.split_at(rem.len() - suffix_len);
+
+            assert_eq!(res_data, data);
+            assert_eq!(res_delim, &[delim]);
+            assert!(res_zeros.iter().all(|&b| b == 0));
+            assert_eq!(res_suffix, suffix);
+        }
+    }
+}
+
 #[test]
 fn test_read() {
     type Buf = ReadBuffer<U4>;
@@ -344,3 +379,57 @@ fn test_read_serialize() {
     let buf = Array([4, 0, 0, 1]);
     assert!(Buf::deserialize(&buf).is_err());
 }
+
+#[test]
+fn eager_buffer_exception_safety() {
+    let mut buf = EagerBuffer::<U4>::default();
+
+    let res = catch_unwind(AssertUnwindSafe(|| {
+        buf.digest_blocks(b"ab", |_| {});
+        buf.digest_blocks(b"cd", |_| panic!("compression panic"));
+    }));
+
+    assert!(res.is_err());
+    let _ = buf.get_pos();
+
+    let mut buf = EagerBuffer::<U4>::default();
+
+    let res = catch_unwind(AssertUnwindSafe(|| {
+        buf.digest_pad(0x80, &[0xFF; 2], |_| panic!("compression panic"));
+    }));
+
+    assert!(res.is_err());
+    let _ = buf.get_pos();
+}
+
+#[test]
+fn read_buffer_exception_safety() {
+    let mut buf = ReadBuffer::<U4>::default();
+
+    let res = catch_unwind(AssertUnwindSafe(|| {
+        buf.write_block(
+            1,
+            |block| {
+                block[0] = 0xFF;
+                panic!("block generation panic");
+            },
+            |_| {},
+        );
+    }));
+
+    assert!(res.is_err());
+    let _ = buf.get_pos();
+
+    let mut buf = ReadBuffer::<U4>::default();
+
+    let res = catch_unwind(AssertUnwindSafe(|| {
+        buf.write_block(
+            1,
+            |block| block.0 = [0xFF; 4],
+            |_| panic!("data read panic"),
+        );
+    }));
+
+    assert!(res.is_err());
+    let _ = buf.get_pos();
+}