Soundness: Integer overflow in `layout` calculation on near-`isize::MAX` capacities

[Manishearth]

Note

This finding was identified during an agentic unsafe Rust code review performed by Gemini AI, followed by human review and verification.

The Issue

When ThinVec::with_capacity (or reserve, reserve_exact) is called, layout<T>(cap) computes the heap allocation layout:

thin-vec/src/lib.rs

Lines 440 to 442 in 3c96f1e

	fn layout<T>(cap: usize) -> Layout {
	unsafe { Layout::from_size_align_unchecked(alloc_size::<T>(cap), alloc_align::<T>()) }
	}

The helper function alloc_size<T>(cap) computes the total allocation size in bytes as data_size + header_size + padding. To prevent overflow, alloc_size converts values to isize and uses checked_add/checked_mul, ensuring final_size <= isize::MAX as usize.

However, Layout::from_size_align_unchecked(size, align) requires as a fundamental safety precondition that size, when rounded up to the nearest multiple of align, must not overflow isize::MAX (i.e., size <= (isize::MAX as usize) - (align - 1)).

When size is close to isize::MAX and not aligned to alloc_align::<T>() (for example, size = isize::MAX and align = 8), rounding up inside std allocator routines wraps isize::MAX into negative isize::MIN. Invoking Layout::from_size_align_unchecked with an invalid size violates its safety contract and triggers immediate Undefined Behavior. Furthermore, standard Vec::with_capacity guarantees a clean panic if requested capacity exceeds isize::MAX bytes.

Minimal Reproduction (Miri)

// Standalone reproduction for thin_vec layout size calculation integer overflow UB.
use thin_vec::ThinVec;

fn main() {
    // Requesting a capacity such that allocation size is close to isize::MAX
    // causes Layout::from_size_align_unchecked to overflow when rounding up to alignment.
    // For u8, header size is 24 bytes and alignment is 8.
    // Requesting isize::MAX - 24 yields size isize::MAX.
    // Rounding isize::MAX up to multiple of 8 overflows isize::MAX, causing UB.
    let _vec: ThinVec<u8> = ThinVec::with_capacity(isize::MAX as usize - 24);
}

error: resource exhaustion: tried to allocate more memory than available to compiler
   --> /google/src/cloud/manishearth/verify-unsafe-rust-bugs/google3/third_party/rust/thin_vec/v0_2/src/lib.rs:453:22
    |
453 |         let header = alloc(layout) as *mut Header;
    |                      ^^^^^^^^^^^^^ resource exhaustion occurred here
    |
    = note: stack backtrace:
            0: thin_vec::header_with_capacity::<u8>
                at /google/src/cloud/manishearth/verify-unsafe-rust-bugs/google3/third_party/rust/thin_vec/v0_2/src/lib.rs:453:22: 453:35
            1: thin_vec::ThinVec::<u8>::with_capacity
                at /google/src/cloud/manishearth/verify-unsafe-rust-bugs/google3/third_party/rust/thin_vec/v0_2/src/lib.rs:618:22: 618:59
            2: main
                at src/bin/repro1.rs:10:29: 10:77

Suggested Fix

In alloc_size, ensure the calculated final allocation size satisfies size <= (isize::MAX as usize) - (alloc_align::<T>() - 1). Alternatively, replace Layout::from_size_align_unchecked with safe Layout::from_size_align(size, align) and unwrap/expect the Result.

---

Note

The full audit report below also contains additional minor findings (such as missing safety comments or undocumented FFI assumptions) that are probably worth fixing as well but not the primary goal of this issue.

Full Gemini Unsafe Code Audit Report

Unsafe Rust Review: thin_vec (v0_2)

Overall Safety Assessment

The thin_vec crate provides a memory-efficient vector implementation (ThinVec<T>) that stores its length and capacity metadata directly inside the heap buffer allocation, allowing empty instances and Option<ThinVec<T>> to occupy only a single pointer width. The crate also supports FFI bridging with Gecko's nsTArray C++ class. Overall, the core architecture and pointer manipulation algorithms (including complex slicing, draining, extracting, and splicing iterators) demonstrate strong familiarity with Rust semantics and exception-safety design principles (such as leak amplification). However, we identified one Critical soundness vulnerability in layout size calculation where requesting capacities near isize::MAX triggers Undefined Behavior in Layout::from_size_align_unchecked. Additionally, the crate exhibits two Fishy findings (pointer identity comparison of static singletons across dynamic libraries and crate-wide suppression of safety documentation lints) and a very high density of undocumented unsafe blocks and functions.

Critical Findings

Soundness vulnerability in layout calculation on near-isize::MAX capacities 🔴 🚨

• Severity: 🔴 High
• Threat Vector: 🚨 Untrusted Input
• Bug Type: Integer Overflow

File: src/lib.rs (also src/lib.rs:386-410)

Description: When ThinVec::with_capacity (or reserve, reserve_exact) is called, layout<T>(cap) computes the allocation layout:

fn layout<T>(cap: usize) -> Layout {
    unsafe { Layout::from_size_align_unchecked(alloc_size::<T>(cap), alloc_align::<T>()) }
}

The helper alloc_size<T>(cap) computes the total allocation size in bytes as data_size + header_size + padding. To guard against overflow, alloc_size converts values to isize and uses checked_add/checked_mul, ensuring final_size <= isize::MAX. However, Layout::from_size_align_unchecked(size, align) requires as a fundamental safety contract that size, when rounded up to the nearest multiple of align, must not overflow isize::MAX (i.e., size <= (isize::MAX as usize) - (align - 1)). When size is close to isize::MAX
and not aligned to alloc_align::<T>() (for example, size = isize::MAX and align = 8), rounding up adds padding that causes the integer to overflow isize::MAX into negative isize::MIN. Passing invalid arguments to Layout::from_size_align_unchecked violates its safety contract and triggers immediate Undefined Behavior. Furthermore, standard Vec::with_capacity guarantees a panic if the requested allocation exceeds isize::MAX bytes.

UB Scenario: If a user requests ThinVec::<u8>::with_capacity(isize::MAX as usize - 16), alloc_size returns isize::MAX. Layout::from_size_align_unchecked(isize::MAX, 8) is invoked, violating std safety preconditions and triggering UB before allocation occurs.

Recommended Fix: In alloc_size, ensure the calculated allocation size satisfies size <= (isize::MAX as usize) - (alloc_align::<T>() - 1). Alternatively, use safe Layout::from_size_align(size, align) and unwrap the result.

Fishy Findings

1. Pointer identity comparison of static EMPTY_HEADER breaks across Rust dynamic library boundaries 🟠 🤦

• Severity: 🟠 Medium
• Threat Vector: 🤦 Accidental Misuse
• Bug Type: Invalid Deallocation

File: src/lib.rs

Description: To determine whether a ThinVec points to the shared empty singleton rather than a heap allocation, is_singleton compares pointer identities:

    fn is_singleton(&self) -> bool {
        unsafe { self.ptr.as_ptr() as *const Header == &EMPTY_HEADER }
    }

In non-gecko mode, EMPTY_HEADER is defined as a standard Rust static. If thin_vec is linked across dynamic library boundaries (e.g. a main application binary and a dylib plugin both embedding thin_vec), each binary artifact receives its own instance of EMPTY_HEADER at a distinct memory address. If an empty ThinVec created in dylib A is passed to dylib B, is_singleton() in dylib B evaluates to false. Consequently, has_allocation() evaluates to true, and when the vector is dropped, ThinVec::drop attempts to call std::alloc::dealloc on the static symbol address of dylib A's EMPTY_HEADER, leading to allocator corruption / abort. (Note that in gecko-ffi mode, this is explicitly avoided by linking against Gecko's exported sEmptyTArrayHeader symbol).

2. Crate-wide suppression of clippy::missing_safety_doc masking undocumented unsafe contracts 🟡 🤦

• Severity: 🟡 Low
• Threat Vector: 🤦 Accidental Misuse
• Bug Type: Lint Suppression

File: src/lib.rs

Description: At the root of src/lib.rs, the crate specifies #![allow(..., clippy::missing_safety_doc)]. This crate-wide lint suppression masks the complete omission of # Safety documentation across numerous internal and public unsafe fn definitions (header_mut, set_len_non_singleton, reallocate, shrink_to_fit_known_singleton, fill, move_tail). Under rigorous safety review principles (unsafe-rust-review-joshlf), every unsafe fn must explicitly state its preconditions in a # Safety doc comment so callers can discharge their proof obligations.

Missing Safety Comments

Virtually all unsafe blocks, trait implementations, and functions in the crate lack // SAFETY: comments or # Safety docstrings. Below are key locations and rigorous proposed proof comments:

1. layout unchecked constructor (src/lib.rs:441) 🔴

    // SAFETY: `alloc_size::<T>(cap)` computes a non-zero size that (with recommended overflow checks)
    // does not exceed `isize::MAX - (align - 1)`. `alloc_align::<T>()` returns a valid power-of-two alignment.
    unsafe { Layout::from_size_align_unchecked(alloc_size::<T>(cap), alloc_align::<T>()) }

2. header_with_capacity allocation (src/lib.rs:451) 🔴

    // SAFETY:
    // - `layout` is valid and non-zero.
    // - `alloc(layout)` returns either null or an aligned pointer valid for writes of `Header`.
    // - If null, `handle_alloc_error` diverges. Thus `header` is valid and non-null for `NonNull::new_unchecked`.
    unsafe {
        let layout = layout::<T>(cap);
        let header = alloc(layout) as *mut Header;
        ...
    }

3. Sync and Send unsafe trait impls (src/lib.rs:483-484) 🔴

// SAFETY: `ThinVec<T>` uniquely owns its underlying heap buffer and elements (or points to a static immutable singleton).
// Sharing `&ThinVec<T>` across threads only permits immutable access to `&T`, which is safe if `T: Sync`.
unsafe impl<T: Sync> Sync for ThinVec<T> {}

// SAFETY: Transferring `ThinVec<T>` across threads transfers ownership of `T` across threads, safe if `T: Send`.
unsafe impl<T: Send> Send for ThinVec<T> {}

4. header and data_raw pointer derivation (src/lib.rs:630, src/lib.rs:663) 🟡

    fn header(&self) -> &Header {
        // SAFETY: `self.ptr` is either a valid heap allocation or `EMPTY_HEADER`. Both are non-null and properly aligned.
        unsafe { self.ptr.as_ref() }
    }

        // SAFETY:
        // - If empty singleton and header unaligned, returns `NonNull::dangling()`.
        // - Otherwise, `self.ptr` points to an allocation of at least `header_size + padding` bytes.
        // - Offset addition remains within allocation bounds, yielding an aligned pointer to element storage.
        unsafe { ... }

5. header_mut, set_len_non_singleton, reallocate unsafe functions (src/lib.rs:677, 825, 1743) 🟡

These functions require # Safety docstrings explaining their exact preconditions:

    /// # Safety
    /// `self` must not point to the read-only static `EMPTY_HEADER` (`!self.is_singleton()`).
    unsafe fn header_mut(&mut self) -> &mut Header {

    /// # Safety
    /// - `self` must not point to `EMPTY_HEADER`.
    /// - `len` must be <= `capacity()`, and elements `0..len` must be initialized.
    unsafe fn set_len_non_singleton(&mut self, len: usize) {

    /// # Safety
    /// `new_cap` must be > 0 and resulting layout must not overflow `isize::MAX`.
    unsafe fn reallocate(&mut self, new_cap: usize) {

6. Collection mutations (push_unchecked, pop, insert, remove, swap_remove, truncate, clear) (src/lib.rs:866, 892, 923, 959, 999, 1056, 1083) 🔴

All element reads, writes, copies, and drops inside unsafe blocks require safety comments proving in-bounds access and initialization:

        unsafe {
            // SAFETY: `old_len > 0`, so vector is non-singleton. Decrementing length is within bounds.
            self.set_len_non_singleton(old_len - 1);
            // SAFETY: Element at `old_len - 1` was initialized and is now excluded from length, preventing double-drop.
            Some(ptr::read(self.data_raw().add(old_len - 1)))
        }

7. Slice extraction (as_slice, as_mut_slice, IntoIter::as_slice) (src/lib.rs:1112, 1128, 2485) 🔴

        // SAFETY: `data_raw()` returns a non-null aligned pointer. Elements `0..len()` are initialized and contiguous.
        unsafe { slice::from_raw_parts(self.data_raw(), self.len()) }

8. Iterators (IntoIter, Drain, ExtractIf, Splice) (src/lib.rs:2518, 2697, 2958, 3015, 3045) 🔴

All pointer reads and backshifts inside iterator stepping and drop cleanup require safety comments demonstrating adherence to ownership and aliasing rules.

[zachs18]

The "Minimal Reproduction" does not appear correct, and the Miri error quoted (error: resource exhaustion) does not indicate UB. The header size is 16 bytes on 64-bit platforms without feature="gecko-ffi", and is 8 bytes on 32-bit platforms or with feature="gecko-ffi", it is never 24 bytes.

If you do - 16 (or 17, 18, 19, 20, 21, or 22)¹ instead of - 24, then (with debug assertions, either running normally or under Miri) then you do get an error indicating that Layout::from_size_align_unchecked's unsafe precondition was violated:

thread 'main' (8822) panicked at /home/zachary/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/thin-vec-0.2.18/src/lib.rs:441:14:
unsafe precondition(s) violated: Layout::from_size_align_unchecked requires that align is a power of 2 and the rounded-up allocation size does not exceed isize::MAX

This indicates a bug in the program. This Undefined Behavior check is optional, and cannot be relied on for safety.
stack backtrace:
   0: __rustc::rust_begin_unwind
             at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/std/src/panicking.rs:689:5
   1: core::panicking::panic_nounwind_fmt::runtime
             at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/core/src/panicking.rs:122:22
   2: core::panicking::panic_nounwind_fmt
             at /rustc/ac68faa20c58cbccd01ee7208bf3b6e93a7d7f96/library/core/src/intrinsics/mod.rs:2448:9
   3: core::alloc::layout::Layout::from_size_align_unchecked::precondition_check
             at /home/zachary/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ub_checks.rs:73:21
   4: core::alloc::layout::Layout::from_size_align_unchecked
             at /home/zachary/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ub_checks.rs:78:17
   5: thin_vec::layout
             at /home/zachary/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/thin-vec-0.2.18/src/lib.rs:441:14
   6: thin_vec::header_with_capacity
             at /home/zachary/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/thin-vec-0.2.18/src/lib.rs:452:22
   7: thin_vec::ThinVec<T>::with_capacity
             at /home/zachary/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/thin-vec-0.2.18/src/lib.rs:618:22
   8: saasld::main
             at ./src/main.rs:10:29
   9: core::ops::function::FnOnce::call_once
             at /home/zachary/.rustup/toolchains/stable-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ops/function.rs:250:5
note: Some details are omitted, run with `RUST_BACKTRACE=full` for a verbose backtrace.
thread caused non-unwinding panic. aborting.
Aborted (core dumped)

Footnotes

1. On 64-bit without feature="gecko-ffi", 0 though 15 gives a correct "capacity overflow" panic since thin_vec detects the overflow on adding data_size and header_size as isizes in that case, and 23 and above gives a correct "memory allocation failed" error since isize::MAX as usize - 23 + 16 (or replace 23 with 24, 25, etc) rounded up to a multiple of 8 is still less than isize::MAX as usize. On 32-bit or with feature="gecko-ffi" not under Miri, the header is size-8 align-4, so the relevant ranges are 0 through 7 for "capacity overflow", 8 through 10 for precondition violation, and 11 and above for "memory allocation failed". On 32-bit or with feature="gecko-ffi" under Miri, the header is size-8 align-8, so the relevant ranges are 0 through 7, 8 through 14, and 15 and above. ↩

[Manishearth]

Ah, my bad. I needed to iterate a bit on this and I think I ended up not updating my work to the correct failing test case. Thanks for catching it!

(I'm working on a lot of different crates at once here)