Soundness: Forming a mutable reference to uninitialized stack memory in Atom::from_mutated_str and push_code_point

[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

In src/lib.rs, the function Atom::from_mutated_str allocates an uninitialized 64-byte stack array via MaybeUninit::<[u8; 64]>::uninit(). It immediately obtains a raw pointer via buffer.as_mut_ptr(), dereferences it, and forms a mutable reference &mut [u8; 64]:

swc/crates/hstr/src/lib.rs

Lines 224 to 225 in 0216889

	let mut buffer = mem::MaybeUninit::<[u8; 64]>::uninit();
	let buffer = unsafe { &mut *buffer.as_mut_ptr() };

According to the docs for std::mem::MaybeUninit, producing a reference (& or &mut) pointing to uninitialized memory is immediate UB. Even though subsequent lines only write to buffer[..s.len()], the remaining 64 - s.len() bytes remain uninitialized while being exposed through a valid reference type &mut [u8; 64].

The compiler and LLVM are permitted to assume that any byte reachable through a reference is fully initialized and valid for its type (u8). This can be triggered from safe public APIs such as Atom::to_ascii_uppercase and Atom::to_ascii_lowercase.

(n.b. the Rust opsem team is considering making this not-UB, but it's not happened yet. Miri does not currently complain about this)

There's a similar issue in push_code_point

swc/crates/hstr/src/wtf8/not_quite_std.rs

Lines 139 to 145 in 0216889

	unsafe {
	// Attempt to not use an intermediate buffer by just pushing bytes
	// directly onto this string.
	let slice = slice::from_raw_parts_mut(string.bytes.as_mut_ptr().add(cur_len), 4);
	let used = encode_utf8_raw(code_point.to_u32(), slice).unwrap_or(0);
	string.bytes.set_len(cur_len + used);
	}

Suggested Fix

Suggested Fix

Replace MaybeUninit with a fully initialized zeroed array on the stack (let mut buffer = [0u8; 64];), which is zero-cost. Alternatively, if MaybeUninit is retained, operate exclusively via raw pointers (*mut u8) or slices of MaybeUninit<u8> (&mut [MaybeUninit<u8>]) without materializing a reference to the uninitialized array.

-        let mut buffer = mem::MaybeUninit::<[u8; 64]>::uninit();
-        let buffer = unsafe { &mut *buffer.as_mut_ptr() };
+        let mut buffer = [0u8; 64];

---

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. The audit report has not been human-reviewed, it may contain misleading claims.

Full Gemini Codebase Audit Report Appendix

Unsafe Rust Review: hstr (v3)

Overall Safety Assessment

hstr is a string interning crate optimized for compilers and build tools written in Rust (such as AST parsers or JavaScript bundlers). It provides immutable string types (Atom and Wtf8Atom) that are cheap to clone, compare, and hash. Internally, strings are represented by a pointer-sized wrapper (TaggedValue) over a non-zero integer or pointer (RawTaggedNonZeroValue). Short strings (up to size_of::<usize>() - 1 bytes, or 7 bytes on 64-bit architectures) are stored inline within the untagged bytes of the pointer representation itself using pointer tagging (low 2 bits indicate tag state). Strings exceeding the inline threshold are interned in thread-local or global reference-counted stores backed by triomphe::ThinArc.

The crate possesses a moderately high density of unsafe code. It relies extensively on manual bit masking, pointer tagging, memory transmutations (std::mem::transmute), reference count manipulation via std::mem::forget, and unchecked UTF-8 / WTF-8 / UTF-16 conversions.

From an architectural standpoint, the core interning design and small-string optimization are conceptually sound when grounded in authoritative Rust layout guarantees (e.g., ThinArc pointer alignment and #[repr(transparent)] layout compatibility between Atom and Wtf8Atom). However, the crate demonstrates poor adherence to unsafe Rust verification practices. Almost none of the unsafe blocks or internal unsafe fn declarations carry explanatory safety proofs (// SAFETY: comments or # Safety docstrings). More critically, the audit uncovered multiple real soundness violations (Critical Findings) where mutable references (&mut [T]) are constructed directly to uninitialized stack buffers (MaybeUninit) and uninitialized Vec spare capacity. Grounded in authoritative Rust Reference rules, producing a reference pointing to uninitialized memory is immediate undefined behavior.

Critical Findings

1. Producing a Mutable Reference to Uninitialized Stack Memory (MaybeUninit) 🔴 🚨

• Priority: 🔴 High
• Threat Vector: 🚨 Untrusted Input
• Bug Type: Uninitialized Memory Reference
• Location: src/lib.rs:224-225

let mut buffer = mem::MaybeUninit::<[u8; 64]>::uninit();
let buffer = unsafe { &mut *buffer.as_mut_ptr() };

• Description: The function Atom::from_mutated_str allocates an uninitialized 64-byte stack array via MaybeUninit::<[u8; 64]>::uninit(). It immediately obtains a raw pointer via buffer.as_mut_ptr() (which returns *mut [u8; 64]), dereferences it, and forms a mutable reference &mut [u8; 64].
• Authoritative Proof Obligation & Soundness Violation: According to the Rust Reference (Behavior Considered Undefined) and standard library documentation for std::mem::MaybeUninit, producing a reference (& or &mut) pointing to uninitialized memory is immediate undefined behavior. Even though subsequent lines only write to buffer[..s.len()], the remaining 64 - s.len() bytes remain uninitialized while being exposed through a valid reference type &mut [u8; 64]. The compiler and LLVM are permitted to assume that any byte reachable through a reference is fully initialized and valid for its type (u8).
• Recommendation: Replace MaybeUninit with a fully initialized zeroed array on the stack (let mut buffer = [0u8; 64];), which is virtually zero-cost. Alternatively, if MaybeUninit is retained, operate exclusively via raw pointers (*mut u8) or slices of MaybeUninit<u8> (&mut [MaybeUninit<u8>]) using MaybeUninit::uninit_array().

2. Producing a Mutable Reference to Uninitialized Vec Spare Capacity 🔴 🚨

• Priority: 🔴 High
• Threat Vector: 🚨 Untrusted Input
• Bug Type: Uninitialized Memory Reference
• Location: src/wtf8/not_quite_std.rs:139-145

    unsafe {
        let slice = slice::from_raw_parts_mut(string.bytes.as_mut_ptr().add(cur_len), 4);
        let used = encode_utf8_raw(code_point.to_u32(), slice).unwrap_or(0);
        string.bytes.set_len(cur_len + used);
    }

• Description: In push_code_point, string.reserve(4) reserves capacity in the underlying Vec<u8>. The memory beyond cur_len up to capacity is uninitialized. The code then calls slice::from_raw_parts_mut(..., 4) to form a &mut [u8] slice pointing to 4 uninitialized bytes in the vector's spare capacity.
• Authoritative Proof Obligation & Soundness Violation: Forming a &[u8] or &mut [u8] slice over uninitialized memory violates Rust's core reference validity invariants and constitutes immediate undefined behavior. slice::from_raw_parts_mut requires that the entire memory range pointed to be initialized for the element type (u8).
• Recommendation: Use string.bytes.spare_capacity_mut() to obtain a &mut [MaybeUninit<u8>] slice without asserting initialization, or encode the code point into a local initialized stack buffer ([0u8; 4]) and append it using string.bytes.extend_from_slice(...).

Fishy Findings

1. Transmuting Wrapped Option<NonNull> Pointers to Extract Pointer Values 🟡 🧪

• Priority: 🟡 Low

• Threat Vector: 🧪 Contrived Setup

• Bug Type: Layout Assumption

• Location: src/tagged_value.rs:104-106, src/tagged_value.rs:111

    pub fn get_ptr(&self) -> *const c_void {
        ...
        unsafe { std::mem::transmute(Some(self.value)) }
    }

    fn get_value(&self) -> RawTaggedValue {
        unsafe { std::mem::transmute(Some(self.value)) }
    }

• Analysis: self.value is of type RawTaggedNonZeroValue (which resolves to std::ptr::NonNull<()> or std::num::NonZeroUsize). To convert this non-zero wrapper into a raw integer or pointer (*const c_void), the code wraps self.value in Some(...) to form an Option<NonNull<()>> and transmutes the Option. While valid due to Rust's niche-filling optimization (where None resolves to 0 and Some(ptr) preserves the exact bit representation of ptr), transmuting Option<NonNull<T>> to *const c_void or usize is an needlessly convoluted idiom. NonNull<T>::as_ptr() or transmuting self.value directly achieves the exact same result without relying on layout assumptions of enum niche representation.

2. Manual Arc Reference Count Increment via restore_arc and mem::forget 🟡 🧪

• Priority: 🟡 Low

• Threat Vector: 🧪 Contrived Setup

• Bug Type: Manual Refcount Manipulation

• Location: src/macros.rs:55-61

                if alias.tag() & TAG_MASK == DYNAMIC_TAG {
                    unsafe {
                        let arc = $crate::dynamic::restore_arc(alias);
                        forget(arc.clone());
                        forget(arc);
                    }
                }

• Analysis: In from_alias (used during Clone), the crate increments the strong count of an interned string by calling restore_arc(alias) to reconstruct an Item(ThinArc(...)) from a raw pointer, cloning the Item, and calling std::mem::forget on both the original and the clone. Because forget suppresses the destructor, the net strong count increases by 1. While sound, reconstructing an Arc wrapper by value and double-forgetting it is brittle and obscure. Standard Arc auditing idioms favor explicit pointer cloning APIs (e.g. ThinArc::clone(&ManuallyDrop::new(restore_arc(alias))) or ThinArc::into_raw).

3. Outdated/Misleading Comment on EntryHasher 🟡 ⚠️

• Priority: 🟡 Low

• Threat Vector: ⚠️ Accidental Misuse

• Bug Type: Documentation Inaccuracy

• Location: src/dynamic.rs:219-223

• Analysis: The documentation comment on EntryHasher states: Assumption: [Arc]'s implementation of [Hash] is a simple pass-through. However, hash table entries in AtomStore are stored as Item(ThinArc<Metadata, u8>). Item implements Hash directly (src/dynamic.rs:37-41) by reading self.0.header.header.hash rather than delegating to ThinArc::hash. The comment is inaccurate and could mislead maintainers auditing hash invariants.

4. Undocumented Unsafe Trait Implementation for rkyv::Archive 🟡 ⚠️

• Priority: 🟡 Low

• Threat Vector: ⚠️ Accidental Misuse

• Bug Type: Missing Safety Documentation

• Location: src/lib.rs:393-396

• Analysis: Atom implements the unsafe trait method rkyv::Archive::resolve. The implementation delegates directly to ArchivedString::resolve_from_str. While sound (since Atom derefs to str and shares string slice semantics), implementing an unsafe serialization trait without documenting the safety contract or proof obligations is bad practice.

Missing Safety Comments

The following file and line locations contain unsafe blocks, unsafe function calls, or unsafe trait/function declarations that lack // SAFETY: proof comments or # Safety docstrings:

1. src/lib.rs:154 — unsafe impl Send for Atom {} 🟡 -Proposed Proof: // SAFETY: Atom is either an inline value or a pointer to an immutable interned string backed by triomphe::ThinArc<Metadata, u8>. Because interned strings are immutable and ThinArc is thread-safe (Send), transferring Atom across thread boundaries cannot introduce data races.
2. src/lib.rs:157 — unsafe impl Sync for Atom {} 🟡 -Proposed Proof: // SAFETY: Atom provides immutable access to string data. Because the underlying storage (inline buffer or ThinArc) is immutable and Sync, concurrent shared references (&Atom) across threads are safe.
3. src/lib.rs:194 — const INLINE_TAG_INIT: NonZeroU8 = unsafe { NonZeroU8::new_unchecked(INLINE_TAG) }; 🔴 -Proposed Proof: // SAFETY: INLINE_TAG is defined as 0b01 (1), which is strictly greater than zero.
4. src/lib.rs:225 — let buffer = unsafe { &mut *buffer.as_mut_ptr() }; 🔴

• (Note: Part of Critical Finding 1. If fixed using raw pointers or MaybeUninit slices, proof should verify pointer bounds and validity).

5. src/lib.rs:229 — let as_str = unsafe { ::std::str::from_utf8_unchecked_mut(buffer_prefix) }; 🔴 -Proposed Proof: // SAFETY: buffer_prefix was initialized via copy_from_slice(s.as_bytes()), where s: &str is guaranteed to be valid UTF-8.
6. src/lib.rs:271 — DYNAMIC_TAG => unsafe { ... } 🔴 -Proposed Proof: // SAFETY: self.tag() == DYNAMIC_TAG guarantees self.unsafe_data holds a valid raw pointer to a live ThinArc<Metadata, u8>. The interned slice was validated as valid UTF-8 upon store insertion. Transmuting the reference lifetime to 'static is sound because the returned &str is bounded by &self.
7. src/lib.rs:278 — unsafe { std::str::from_utf8_unchecked(&src[..(len as usize)]) } 🔴 -Proposed Proof: // SAFETY: self.tag() == INLINE_TAG guarantees that the untagged data bytes up to len were initialized from a valid UTF-8 &str.
8. src/lib.rs:280 — _ => unsafe { debug_unreachable!() }, 🔴 -Proposed Proof: // SAFETY: self.tag() is masked by TAG_MASK (0b11). Only DYNAMIC_TAG (0b00) and INLINE_TAG (0b01) are constructed by Atom.
9. src/lib.rs:290 — let ptr = unsafe { crate::dynamic::deref_from(self.unsafe_data) }; 🔴 -Proposed Proof: // SAFETY: Matching on DYNAMIC_TAG ensures self.unsafe_data contains a valid non-null pointer to an interned ThinArc.
10. src/lib.rs:323 — unsafe { drop(crate::dynamic::restore_arc(self.unsafe_data)) } 🔴 -Proposed Proof: // SAFETY: self.is_dynamic() checks that tag == DYNAMIC_TAG. Reconstructing the ThinArc and dropping it safely releases one strong reference count owned by this Atom.
11. src/lib.rs:394 — unsafe fn resolve(&self, pos: usize, resolver: Self::Resolver, out: *mut Self::Archived) 🔴 -Proposed Proof: // SAFETY: Atom dereferences to str. Delegating resolve to ArchivedString::resolve_from_str satisfies the rkyv Archive contract.
12. src/wtf8_atom.rs:70 — unsafe impl Send for Wtf8Atom {} 🟡 -Proposed Proof: // SAFETY: Wtf8Atom wraps TaggedValue pointing to immutable inline bytes or an immutable ThinArc. ThinArc and TaggedValue are safe to transfer across threads.
13. src/wtf8_atom.rs:73 — unsafe impl Sync for Wtf8Atom {} 🟡 -Proposed Proof: // SAFETY: Wtf8Atom provides immutable access to well-formed WTF-8 data. Shared references (&Wtf8Atom) are thread-safe.
14. src/wtf8_atom.rs:193 — result.push(unsafe { CodePoint::from_u32_unchecked(code_point as u32) }); 🔴 -Proposed Proof: // SAFETY: code_point is parsed as u16, so code_point as u32 <= 0xFFFF, which is within the valid Unicode code point range (<= 0x10FFFF).
15. src/wtf8_atom.rs:264 — unsafe { drop(crate::dynamic::restore_arc(self.unsafe_data)) } 🔴 -Proposed Proof: // SAFETY: self.is_dynamic() ensures tag == DYNAMIC_TAG. Reconstructing and dropping the ThinArc releases one owned strong reference.
16. src/wtf8_atom.rs:341 — DYNAMIC_TAG => unsafe { ... } 🔴 -Proposed Proof: // SAFETY: DYNAMIC_TAG ensures unsafe_data points to a live interned ThinArc whose slice contains well-formed WTF-8.
17. src/wtf8_atom.rs:348 — unsafe { Wtf8::from_bytes_unchecked(&src[..(len as usize)]) } 🔴 -Proposed Proof: // SAFETY: INLINE_TAG ensures data bytes up to len contain well-formed WTF-8 copied during atom creation.
18. src/wtf8_atom.rs:350 — _ => unsafe { debug_unreachable!() }, 🔴 -Proposed Proof: // SAFETY: Tag bits are masked by TAG_MASK (0b11); only 0b00 and 0b01 are valid states.
19. src/wtf8_atom.rs:362 — let ptr = unsafe { crate::dynamic::deref_from(self.unsafe_data) }; 🔴 -Proposed Proof: // SAFETY: Checked under DYNAMIC_TAG branch.
20. src/wtf8_atom.rs:394, 406, 419 — wtf8.push(unsafe { CodePoint::from_u32_unchecked(...) }); (in unit tests) 🔴 -Proposed Proof: // SAFETY: Literal constants 0xD800 and 0xDC00 are <= 0x10FFFF.
21. src/dynamic.rs:43 — pub(crate) unsafe fn deref_from(ptr: TaggedValue) -> ManuallyDrop<Item> 🔴 -Proposed Doc Proof: /// # Safety\n/// Caller must ensure ptr has DYNAMIC_TAG and holds a non-null pointer to a live interned ThinArc.
22. src/dynamic.rs:49 — pub(crate) unsafe fn restore_arc(v: TaggedValue) -> Item 🔴 -Proposed Doc Proof: /// # Safety\n/// Caller must ensure v holds a valid pointer to a live ThinArc and transfers one strong reference count ownership to the returned Item.
23. src/dynamic.rs:129 — unsafe { unsafe_data.data_mut()[..len].copy_from_slice(text); } 🔴 -Proposed Proof: // SAFETY: len <= MAX_INLINE_LEN (7) guarantees ..len is within the 8-byte TaggedValue inline allocation.
24. src/dynamic.rs:169 — unsafe { let data = unsafe_data.data_mut(); ... } 🔴 -Proposed Proof: // SAFETY: unsafe_data was initialized with a non-zero tag byte, satisfying data_mut preconditions.
25. src/macros.rs:6 — unsafe { $crate::dynamic::deref_from(unsafe_data) } 🔴 -Proposed Proof: // SAFETY: DYNAMIC_TAG branch guarantees unsafe_data holds a valid pointer to an interned string Arc.
26. src/macros.rs:16 — _ => unsafe { debug_unreachable!() }, 🔴 -Proposed Proof: // SAFETY: Only DYNAMIC_TAG and INLINE_TAG are valid tag states.
27. src/macros.rs:34-35 — let te = unsafe { ... }; let oe = unsafe { ... }; 🔴 -Proposed Proof: // SAFETY: Both self and other checked is_dynamic() == true, ensuring valid Arc pointers.
28. src/macros.rs:56 — unsafe { let arc = ...; forget(...); forget(...); } 🔴 -Proposed Proof: // SAFETY: DYNAMIC_TAG ensures alias holds a live Arc pointer. Reconstructing and double-forgetting increments strong count by 1.
29. src/tagged_value.rs:54 — unsafe { let value: ... = transmute(value); ... } 🔴 -Proposed Proof: // SAFETY: NonNull<T> is non-null and layout-compatible with NonZeroUsize.
30. src/tagged_value.rs:78 — value: unsafe { std::mem::transmute(value) } 🔴 -Proposed Proof: // SAFETY: value is NonZeroU8 cast to RawTaggedValue, preserving non-zero low byte required for RawTaggedNonZeroValue niche.
31. src/tagged_value.rs:104, 111 — unsafe { std::mem::transmute(Some(self.value)) } 🔴 -Proposed Proof: // SAFETY: Option<RawTaggedNonZeroValue> has the exact same size and ABI representation as RawTaggedValue / *const c_void via niche optimization.
32. src/tagged_value.rs:125, 130 — unsafe { data = data.offset(1); } ... unsafe { slice::from_raw_parts(data, len) } 🔴 -Proposed Proof: // SAFETY: TaggedValue is 8 bytes. Offsetting by 1 byte on little-endian and slicing len (7) bytes stays within allocated object bounds.
33. src/wtf8/mod.rs:109 — _ => Some(unsafe { char::from_u32_unchecked(self.value) }), 🔴 -Proposed Proof: // SAFETY: CodePoint guarantees value <= 0x10FFFF, and match arm excludes surrogates (0xD800..=0xDFFF), meeting char validity requirements.
34. src/wtf8/mod.rs:148 — unsafe { transmute(&*self.bytes) } 🔴 -Proposed Proof: // SAFETY: Wtf8 is #[repr(transparent)] over [u8], and Wtf8Buf maintains well-formed WTF-8 invariants.
35. src/wtf8/mod.rs:250 — let code_point = unsafe { CodePoint::from_u32_unchecked(s as u32) }; 🟡 -Proposed Proof: // SAFETY: Decoded UTF-16 surrogate code units (0xD800..=0xDFFF) are <= 0x10FFFF.
36. src/wtf8/mod.rs:360 — None => Ok(unsafe { String::from_utf8_unchecked(self.bytes) }), 🔴 -Proposed Proof: // SAFETY: next_surrogate(0) == None confirms no surrogates exist in self.bytes. In well-formed WTF-8, lack of surrogates implies valid UTF-8.
37. src/wtf8/mod.rs:379 — None => return unsafe { String::from_utf8_unchecked(self.bytes) }, 🔴 -Proposed Proof: // SAFETY: All surrogates were replaced with valid UTF-8 replacement characters, leaving self.bytes valid UTF-8.
38. src/wtf8/mod.rs:469, 477 — formatter.write_str(unsafe { str::from_utf8_unchecked(...) }) 🔴 -Proposed Proof: // SAFETY: Subslice between pos and surrogate_pos contains no surrogates and lies on character boundaries, guaranteeing valid UTF-8.
39. src/wtf8/mod.rs:488 — unsafe { transmute(value.as_bytes()) } 🟡 -Proposed Proof: // SAFETY: Valid UTF-8 &str is well-formed WTF-8, and Wtf8 is #[repr(transparent)] over [u8].
40. src/wtf8/not_quite_std.rs:139 — unsafe { let slice = slice::from_raw_parts_mut(...); ... } 🔴

• (Note: Part of Critical Finding 2).

41. src/wtf8/not_quite_std.rs:162 — pub unsafe fn slice_unchecked(s: &Wtf8, begin: usize, end: usize) -> &Wtf8 🔴 -Proposed Doc Proof: /// # Safety\n/// Caller must ensure begin <= end, end <= s.len(), and both lie on code point boundaries.
42. src/wtf8/not_quite_std.rs:224, 243 — unsafe { char::from_u32_unchecked(...) } 🟡 -Proposed Proof: // SAFETY: Decoded non-surrogate code units or calculated surrogate pair scalar values are valid Unicode scalar values.
43. src/tests.rs:182 — let atom2 = unsafe { Atom::from_wtf8_unchecked(wtf8.clone()) }; (in unit tests) 🔴 -Proposed Proof: // SAFETY: Test string wtf8 was constructed without unpaired surrogates.