dimforge / dimforge/nalgebra

Panic-safety unsoundness in Matrix editing methods (double-free / use-after-free when an element's Drop panics)

Open
#1,615 0 comments 0 reactions 0 assignees View on GitHub
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

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.