Panic-safety unsoundness in Matrix editing methods (double-free / use-after-free when an element's Drop panics)
- Dominant language
- Rust
- Stars
- 4.8k
- Forks
- 565
- PR merge metrics
- No merged PRs in 30d
Description
Hello, and thanks for your work on nalgebra. While studying `Drop` behavior in a
few Rust crates, I found a panic-safety soundness issue in several `Matrix` editing
methods, and I'd like to report it here.
## The problem
Several methods in `src/base/edition.rs` call `ptr::drop_in_place` on elements
*before* the metadata/buffer update that removes them from `VecStorage`. Because
`drop_in_place` runs the element's user-provided `Drop`, a user can supply a `Drop`
that panics. When it does, the update after the drop is skipped, so `VecStorage`
still considers those slots live and drops them again — a use-after-free / double
free when the `Matrix` is dropped (or when the panic is caught with
`std::panic::catch_unwind`). This is reachable from safe Rust, so it's a soundness
issue.
## Where it happens
Six sites across five methods (`resize_generic` has two). Representative case,
`remove_columns_at`:
```rust
unsafe { ptr::drop_in_place(col_slice); } // runs the element's Drop — may panic
offset += 1; // skipped on panic → slots left "live"
```
- Direct `drop_in_place` before a metadata update: `remove_columns_at`,
`remove_columns_generic`, `remove_rows_at`, `resize_generic` (column branch).
- Via `unsafe fn compress_rows` (drops in a loop, no panic guard):
`resize_generic` (row branch), `remove_rows_generic`.
## Suggested fix
Perform the metadata/buffer update *before* dropping the elements (or drop behind a
guard that commits progress on unwind), so the storage never re-drops an element
whose `Drop` already ran.
## Versions
Originally found on 0.34.2; also confirmed on 0.35.0 (latest).
## How I'm reporting this
I first reported this privately via a GitHub Security Advisory on 2026-04-23
(GHSA-wcph-xf53-94vr) and followed up on 2026-05-12, but it's stayed in Triage
without a response for ~3 months. I'm opening this so it's tracked publicly and
downstream users are aware. Thank you for your time.
Contributor guide
No contributing guide indexed for this repository
Research direction
Start in src/base/edition.rs and inspect resize_generic, remove_columns_at, remove_columns_generic, remove_rows_at, remove_rows_generic, and unsafe compress_rows, focusing on the order of drop_in_place and metadata or buffer updates. Reproduce the panic path with an element whose Drop panics and catch_unwind; done means storage does not drop an already-dropped element again.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- security
- Issue type
- Bug
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Quiet
- Clarity
- Clearly specified
- Newbie friendliness
- 42/100