-
Notifications
You must be signed in to change notification settings - Fork 156
Open
Description
Hello, I found a soundness issue in cgmath 0.18.0 affecting three implementations of swap_columns.
Summary
Matrix2::swap_columns, Matrix3::swap_columns, and Matrix4::swap_columns all use:
unsafe { ptr::swap(&mut self[a], &mut self[b]) };When a == b, this creates two overlapping mutable references to the same location through a Safe API call, violating Rust aliasing guarantees and triggering UB under Miri.
Why this is unsound:
- Caller uses only Safe Rust.
- Passing equal indices is allowed by the function signature.
- Equal indices cause aliased &mut borrows.
- Miri reports UB (Stacked Borrows violation) in
std::ptr::swap, with backtrace into each corresponding swap_columns implementation.
Minimal PoCs (Safe Rust):
- Matrix2:
use cgmath::{Matrix, Matrix2};
fn main() {
let mut m = Matrix2::new(1.0, 2.0, 3.0, 4.0);
m.swap_columns(0, 0);
}- Matrix3:
use cgmath::{Matrix, Matrix3};
fn main() {
let mut m = Matrix3::new(
1.0, 2.0, 3.0,
4.0, 5.0, 6.0,
7.0, 8.0, 9.0,
);
m.swap_columns(1, 1);
}- Matrix4:
use cgmath::{Matrix, Matrix4};
fn main() {
let mut m = Matrix4::new(
1.0, 2.0, 3.0, 4.0,
5.0, 6.0, 7.0, 8.0,
9.0, 10.0, 11.0, 12.0,
13.0, 14.0, 15.0, 16.0,
);
m.swap_columns(2, 2);
}Miri report:
- Matrix2:
error: Undefined Behavior: attempting a read access using <281> at alloc120[0x0], but that tag does not exist in the borrow stack for this location
--> /home/yilin/.rustup/toolchains/nightly-2025-12-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:1308:9
|
1308 | copy_nonoverlapping(x, tmp.as_mut_ptr(), 1);
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ this error occurs as part of an access at alloc120[0x0..0x10]
|
= help: this indicates a potential bug in the program: it performed an invalid operation, but the Stacked Borrows rules it violated are still experimental
= help: see https://github.com/rust-lang/unsafe-code-guidelines/blob/master/wip/stacked-borrows.md for further information
help: <281> was created by a SharedReadWrite retag at offsets [0x0..0x10]
--> src/main.rs:9:5
|
9 | m.swap_columns(0, 0);
| ^^^^^^^^^^^^^^^^^^^^
help: <281> was later invalidated at offsets [0x0..0x20] by a Unique retag
--> src/main.rs:9:5
|
9 | m.swap_columns(0, 0);
| ^^^^^^^^^^^^^^^^^^^^
= note: BACKTRACE (of the first span):
= note: inside `std::ptr::swap::<cgmath::Vector2<f64>>` at /home/yilin/.rustup/toolchains/nightly-2025-12-06-x86_64-unknown-linux-gnu/lib/rustlib/src/rust/library/core/src/ptr/mod.rs:1308:9: 1308:52
= note: inside `<cgmath::Matrix2<f64> as cgmath::Matrix>::swap_columns` at /home/yilin/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/cgmath-0.18.0/src/matrix.rs:583:18: 583:55
note: inside `main`
--> src/main.rs:9:5
|
9 | m.swap_columns(0, 0);
| ^^^^^^^^^^^^^^^^^^^^
note: some details are omitted, run with `MIRIFLAGS=-Zmiri-backtrace=full` for a verbose backtrace
error: aborting due to 1 previous errorThe miri report of Matirx3 and Matrix4 are same to Matrix2.
Suggested fix:
A minimal safe fix is to return early when indices are equal:
if a == b {
return;
}
unsafe { ptr::swap(&mut self[a], &mut self[b]) };This should be applied consistently to all three implementations.
Thanks for maintaining cgmath.
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels