Skip to content

Panic-safety unsoundness in several collections (double-free / use-after-free) #8

Description

@tooson9010-spec

Hello — while researching panic-safety in Rust crates, I found that several public
APIs in coca 0.3.0 are not panic-safe. Each is reachable from safe Rust and can
lead to double-free (CWE-415) / use-after-free (CWE-416) or a read of uninitialized
memory (CWE-908). There are three distinct root causes.


Class 1 — destruction before the metadata commit (double-free)

The affected methods destroy initialized values with drop_in_place and only
afterwards commit the metadata that removes those values from the collection's
logical state (logical length, occupancy flag, generation / free-list, etc.). A
destructor is user-controlled and may panic; if it does, the metadata update is
skipped and the collection still treats the already-destroyed values as live. When
the collection is later dropped, those values are dropped a second time.

Representative shape (Deque::truncate, Vec::truncate):

drop_removed_elements(); // an element's Drop may panic
self.len = new_len;      // skipped on unwind -> stale len

Affected methods:

  • UnitCache::clear
  • LruCache2::get_or_insert_with
  • Deque::truncate, Deque::clear, Deque::retain
  • ListMap::clear, ListMap::remove
  • PackedPool::clear
  • Vec::truncate

Class 2 — metadata commit before initialization (uninitialized read)

try_insert_with_handle marks a slot occupied / bumps the element count before
running the user-supplied filler closure. If the closure unwinds without producing
a value, the slot is left uninitialized while the metadata already claims it holds a
live element. A later read or the pool's own Drop then touches that uninitialized
slot.

// slot marked occupied / count bumped here ...
let value = filler(handle); // may unwind before producing a value
// ... slot stays uninitialized, but metadata says it is live

Affected method:

  • DirectPool::try_insert_with_handle

Confirmed under Miri: after a panicking filler, a subsequent access reports
reading memory ... but memory is uninitialized.


Class 3 — non-destructive compaction before the metadata commit (Deque::retain)

Deque::retain compacts kept elements toward the front with ptr::copy and commits
self.len only after the loop. ptr::copy does not invalidate the source slot, so a
compacted element exists in two slots that are both inside the current logical
length. If the predicate unwinds after a compaction, the length commit is skipped and
the duplicate stays live — dropped a second time on Drop.

for i in 0..old_len {
    let src = ptr_at_index(&self.buf, i % capacity);
    if f(unsafe { &*src }) {                          // predicate may unwind
        let dst = mut_ptr_at_index(&mut self.buf, new_len % capacity);
        unsafe { core::ptr::copy(src, dst, 1); }      // duplicates the value
        new_len += 1;
    } else {
        unsafe { core::ptr::drop_in_place(mut_ptr_at_index(&mut self.buf, i % capacity)); }
    }
}
self.len = I::from_usize(new_len);                    // skipped on unwind

Affected method:

  • Deque::retain (src/collections/deque.rs:827)

Unlike Class 1, no element Drop needs to panic here — the predicate alone is
enough (unwrap(), indexing, assert!). (Deque::retain also has the Class 1 path
when a removed element's Drop panics at drop_in_place.)


Trigger conditions

  • Class 1: an element/key/value whose Drop can panic, plus a call to the affected method.
  • Class 2: a filler closure that unwinds before producing a value.
  • Class 3: any element that owns memory, plus a predicate that can panic (no panicking Drop needed).

All are reachable from safe Rust with unwinding panics and catch_unwind.

Affected versions

Confirmed on coca 0.3.0.

Thank you for your time.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions