Skip to content

Commit 2874984

Browse files
AxectAxect
committed
SECURITY: Encapsulate Matrix / ComplexMatrix fields (#101)
Safe code could construct a Matrix whose data.len() disagrees with row * col by writing the public fields directly, and the internal unsafe code (raw pointers via ptr(), matrixmultiply / BLAS dgemm calls, Index arithmetic) trusts those fields without checking the buffer length, so heap out-of-bounds reads and writes were reachable without any unsafe in user code (reported in #101 for peroxide 0.41.0). Root fix, targeted at v0.42.0 (breaking change): * Matrix and ComplexMatrix fields are now pub(crate); the invariant data.len() == row * col can no longer be broken from outside the crate. * The matrix() / cmatrix() constructors assert the invariant, so the remaining public construction path is validated. * New public accessors replace direct field reads: nrow(), ncol(), layout() (storage order), into_vec() (consume into the buffer). Reading the buffer stays available through as_slice() / as_mut_slice(); dimensions also via MatrixTrait::shape(). * cbind! / rbind! expanded to field accesses at the caller's site and would no longer compile for downstream users; rewritten with the public accessors. * Struct-literal doc examples replaced with constructor calls; tests and examples migrated. Known follow-up: serde / rkyv deserialization can still bypass the constructor validation; tracked separately. Migration for downstream code: m.row -> m.nrow() m.col -> m.ncol() m.shape -> m.layout() m.data -> m.as_slice() / as_mut_slice() / into_vec() Matrix { .. } literal -> matrix(data, row, col, shape)
1 parent fc98926 commit 2874984

7 files changed

Lines changed: 160 additions & 82 deletions

File tree

‎examples/clippy_verify.rs‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -39,11 +39,11 @@ fn hash_f64s(v: &[f64]) -> String {
3939

4040
fn hash_matrix(m: &Matrix) -> String {
4141
let mut h = DefaultHasher::new();
42-
m.row.hash(&mut h);
43-
m.col.hash(&mut h);
44-
// m.shape is an enum without Hash; use Debug as a stable surrogate
45-
format!("{:?}", m.shape).hash(&mut h);
46-
for x in &m.data {
42+
m.nrow().hash(&mut h);
43+
m.ncol().hash(&mut h);
44+
// the layout enum has no Hash impl; use Debug as a stable surrogate
45+
format!("{:?}", m.layout()).hash(&mut h);
46+
for x in m.as_slice() {
4747
x.to_bits().hash(&mut h);
4848
}
4949
format!("{:016x}", h.finish())

‎src/complex/matrix.rs‎

Lines changed: 52 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -25,30 +25,55 @@ use crate::{
2525

2626
/// R-like complex matrix structure
2727
///
28+
/// The fields are private to protect the invariant `data.len() == row * col`,
29+
/// which the internal unsafe code relies on. Construct complex matrices with
30+
/// [`cmatrix`] and friends, and read the dimensions back with
31+
/// [`ComplexMatrix::nrow`], [`ComplexMatrix::ncol`], and
32+
/// [`ComplexMatrix::layout`].
33+
///
2834
/// # Examples
2935
///
3036
/// ```rust
3137
/// use peroxide::fuga::*;
32-
/// use peroxide::complex::matrix::ComplexMatrix;
38+
/// use peroxide::complex::matrix::cmatrix;
3339
///
34-
/// let v1 = ComplexMatrix {
35-
/// data: vec![
40+
/// let v1 = cmatrix(vec![
3641
/// C64::new(1f64, 1f64),
3742
/// C64::new(2f64, 2f64),
3843
/// C64::new(3f64, 3f64),
3944
/// C64::new(4f64, 4f64),
40-
/// ],
41-
/// row: 2,
42-
/// col: 2,
43-
/// shape: Row,
44-
/// }; // [[1+1i,2+2i],[3+3i,4+4i]]
45+
/// ], 2, 2, Row); // [[1+1i,2+2i],[3+3i,4+4i]]
46+
/// assert_eq!(v1.nrow(), 2);
47+
/// assert_eq!(v1.ncol(), 2);
4548
/// ```
4649
#[derive(Debug, Clone, Default)]
4750
pub struct ComplexMatrix {
48-
pub data: Vec<C64>,
49-
pub row: usize,
50-
pub col: usize,
51-
pub shape: Shape,
51+
pub(crate) data: Vec<C64>,
52+
pub(crate) row: usize,
53+
pub(crate) col: usize,
54+
pub(crate) shape: Shape,
55+
}
56+
57+
impl ComplexMatrix {
58+
/// Number of rows
59+
pub fn nrow(&self) -> usize {
60+
self.row
61+
}
62+
63+
/// Number of columns
64+
pub fn ncol(&self) -> usize {
65+
self.col
66+
}
67+
68+
/// Storage order of the underlying buffer (row-major or column-major)
69+
pub fn layout(&self) -> Shape {
70+
self.shape
71+
}
72+
73+
/// Consume the matrix and return the underlying buffer in storage order
74+
pub fn into_vec(self) -> Vec<C64> {
75+
self.data
76+
}
5277
}
5378

5479
// =============================================================================
@@ -70,14 +95,23 @@ pub struct ComplexMatrix {
7095
/// C64::new(4f64, 4f64)],
7196
/// 2, 2, Row
7297
/// );
73-
/// a.col.print(); // Print matrix column
98+
/// a.ncol().print(); // Print number of columns
7499
/// ```
75100
pub fn cmatrix<T>(v: Vec<T>, r: usize, c: usize, shape: Shape) -> ComplexMatrix
76101
where
77102
T: Into<C64>,
78103
{
104+
let data = v.into_iter().map(|t| t.into()).collect::<Vec<C64>>();
105+
assert_eq!(
106+
data.len(),
107+
r * c,
108+
"cmatrix: data length ({}) does not match row * col ({} * {})",
109+
data.len(),
110+
r,
111+
c
112+
);
79113
ComplexMatrix {
80-
data: v.into_iter().map(|t| t.into()).collect::<Vec<C64>>(),
114+
data,
81115
row: r,
82116
col: c,
83117
shape,
@@ -271,9 +305,9 @@ impl MatrixTrait for ComplexMatrix {
271305
/// C64::new(4f64, 4f64)],
272306
/// 2, 2, Row
273307
/// );
274-
/// assert_eq!(a.shape, Row);
308+
/// assert_eq!(a.layout(), Row);
275309
/// let b = a.change_shape();
276-
/// assert_eq!(b.shape, Col);
310+
/// assert_eq!(b.layout(), Col);
277311
/// ```
278312
fn change_shape(&self) -> Self {
279313
let r = self.row;
@@ -320,9 +354,9 @@ impl MatrixTrait for ComplexMatrix {
320354
/// ],
321355
/// 2, 2, Row
322356
/// );
323-
/// assert_eq!(a.shape, Row);
357+
/// assert_eq!(a.layout(), Row);
324358
/// a.change_shape_mut();
325-
/// assert_eq!(a.shape, Col);
359+
/// assert_eq!(a.layout(), Col);
326360
/// ```
327361
fn change_shape_mut(&mut self) {
328362
let r = self.row;

‎src/macros/r_macro.rs‎

Lines changed: 31 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -144,17 +144,17 @@ macro_rules! cbind {
144144
( $x:expr, $y:expr ) => {
145145
{
146146
let mut temp = $x;
147-
if temp.shape != Col {
147+
if temp.layout() != Col {
148148
temp = temp.change_shape();
149149
}
150150

151-
let mut v: Vec<f64> = temp.data;
152-
let mut c: usize = temp.col;
153-
let r: usize = temp.row;
151+
let r: usize = temp.nrow();
152+
let mut c: usize = temp.ncol();
153+
let mut v: Vec<f64> = temp.into_vec();
154154

155-
assert_eq!(r, $y.row);
156-
v.extend(&$y.data.clone());
157-
c += &$y.col;
155+
assert_eq!(r, $y.nrow());
156+
v.extend($y.as_slice());
157+
c += $y.ncol();
158158
matrix(v, r, c, Col)
159159
}
160160
};
@@ -163,22 +163,22 @@ macro_rules! cbind {
163163
( $x0:expr, $( $x: expr ),* ) => {
164164
{
165165
let mut temp0 = $x0;
166-
if temp0.shape != Col {
166+
if temp0.layout() != Col {
167167
temp0 = temp0.change_shape();
168168
}
169-
let mut v: Vec<f64> = temp0.data;
170-
let mut c: usize = temp0.col;
171-
let r: usize = temp0.row;
169+
let r: usize = temp0.nrow();
170+
let mut c: usize = temp0.ncol();
171+
let mut v: Vec<f64> = temp0.into_vec();
172172
$(
173173
let mut temp = $x;
174-
if temp.shape != Col {
174+
if temp.layout() != Col {
175175
temp = temp.change_shape();
176176
}
177177
// Must equal row
178-
assert_eq!(r, temp.row);
178+
assert_eq!(r, temp.nrow());
179179
// Add column
180-
c += temp.col;
181-
v.extend(&temp.data.clone());
180+
c += temp.ncol();
181+
v.extend(temp.as_slice());
182182
)*
183183
matrix(v, r, c, Col)
184184
}
@@ -207,22 +207,22 @@ macro_rules! rbind {
207207
( $x:expr, $y:expr ) => {
208208
{
209209
let mut temp = $x;
210-
if temp.shape != Row {
210+
if temp.layout() != Row {
211211
temp = temp.change_shape();
212212
}
213213

214214
let mut temp2 = $y;
215-
if temp2.shape != Row {
215+
if temp2.layout() != Row {
216216
temp2 = temp2.change_shape();
217217
}
218218

219-
let mut v: Vec<f64> = temp.data;
220-
let c: usize = temp.col;
221-
let mut r: usize = temp.row;
219+
let c: usize = temp.ncol();
220+
let mut r: usize = temp.nrow();
221+
let mut v: Vec<f64> = temp.into_vec();
222222

223-
assert_eq!(c, temp2.col);
224-
v.extend(&temp2.data.clone());
225-
r += temp2.row;
223+
assert_eq!(c, temp2.ncol());
224+
r += temp2.nrow();
225+
v.extend(temp2.as_slice());
226226
matrix(v, r, c, Row)
227227
}
228228
};
@@ -231,22 +231,22 @@ macro_rules! rbind {
231231
( $x0:expr, $( $x: expr ),* ) => {
232232
{
233233
let mut temp0 = $x0;
234-
if temp0.shape != Row {
234+
if temp0.layout() != Row {
235235
temp0 = temp0.change_shape();
236236
}
237-
let mut v: Vec<f64> = temp0.data;
238-
let c: usize = temp0.col;
239-
let mut r: usize = temp0.row;
237+
let c: usize = temp0.ncol();
238+
let mut r: usize = temp0.nrow();
239+
let mut v: Vec<f64> = temp0.into_vec();
240240
$(
241241
let mut temp = $x;
242-
if temp.shape != Row {
242+
if temp.layout() != Row {
243243
temp = temp.change_shape();
244244
}
245245
// Must equal row
246-
assert_eq!(c, temp.col);
246+
assert_eq!(c, temp.ncol());
247247
// Add column
248-
r += temp.row;
249-
v.extend(&temp.data.clone());
248+
r += temp.nrow();
249+
v.extend(temp.as_slice());
250250
)*
251251
matrix(v, r, c, Row)
252252
}

‎src/structure/matrix.rs‎

Lines changed: 51 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -678,17 +678,22 @@ impl fmt::Display for Shape {
678678

679679
/// R-like matrix structure
680680
///
681+
/// The fields are private to protect the invariant `data.len() == row * col`,
682+
/// which the internal unsafe code (raw pointers, BLAS calls) relies on.
683+
/// Construct matrices with [`matrix`], the `ml_matrix!` / `py_matrix!` style
684+
/// constructors, or [`MatrixTrait::from_index`], and read the dimensions back
685+
/// with [`Matrix::nrow`], [`Matrix::ncol`], [`MatrixTrait::shape`], and
686+
/// [`Matrix::layout`].
687+
///
681688
/// # Examples
682689
///
683690
/// ```
684691
/// use peroxide::fuga::*;
685692
///
686-
/// let a = Matrix {
687-
/// data: vec![1f64,2f64,3f64,4f64],
688-
/// row: 2,
689-
/// col: 2,
690-
/// shape: Row,
691-
/// }; // [[1,2],[3,4]]
693+
/// let a = matrix(vec![1f64, 2f64, 3f64, 4f64], 2, 2, Row); // [[1,2],[3,4]]
694+
/// assert_eq!(a.nrow(), 2);
695+
/// assert_eq!(a.ncol(), 2);
696+
/// assert_eq!(a.layout(), Row);
692697
/// ```
693698
#[derive(Debug, Clone, Default)]
694699
#[cfg_attr(feature = "serde", derive(Serialize, Deserialize))]
@@ -697,10 +702,32 @@ impl fmt::Display for Shape {
697702
derive(rkyv::Archive, rkyv::Serialize, rkyv::Deserialize)
698703
)]
699704
pub struct Matrix {
700-
pub data: Vec<f64>,
701-
pub row: usize,
702-
pub col: usize,
703-
pub shape: Shape,
705+
pub(crate) data: Vec<f64>,
706+
pub(crate) row: usize,
707+
pub(crate) col: usize,
708+
pub(crate) shape: Shape,
709+
}
710+
711+
impl Matrix {
712+
/// Number of rows
713+
pub fn nrow(&self) -> usize {
714+
self.row
715+
}
716+
717+
/// Number of columns
718+
pub fn ncol(&self) -> usize {
719+
self.col
720+
}
721+
722+
/// Storage order of the underlying buffer (row-major or column-major)
723+
pub fn layout(&self) -> Shape {
724+
self.shape
725+
}
726+
727+
/// Consume the matrix and return the underlying buffer in storage order
728+
pub fn into_vec(self) -> Vec<f64> {
729+
self.data
730+
}
704731
}
705732

706733
// =============================================================================
@@ -724,8 +751,17 @@ pub fn matrix<T>(v: Vec<T>, r: usize, c: usize, shape: Shape) -> Matrix
724751
where
725752
T: Into<f64>,
726753
{
754+
let data = v.into_iter().map(|t| t.into()).collect::<Vec<f64>>();
755+
assert_eq!(
756+
data.len(),
757+
r * c,
758+
"matrix: data length ({}) does not match row * col ({} * {})",
759+
data.len(),
760+
r,
761+
c
762+
);
727763
Matrix {
728-
data: v.into_iter().map(|t| t.into()).collect::<Vec<f64>>(),
764+
data,
729765
row: r,
730766
col: c,
731767
shape,
@@ -879,9 +915,9 @@ impl MatrixTrait for Matrix {
879915
/// use peroxide::fuga::*;
880916
///
881917
/// let a = matrix(vec![1,2,3,4],2,2,Row);
882-
/// assert_eq!(a.shape, Row);
918+
/// assert_eq!(a.layout(), Row);
883919
/// let b = a.change_shape();
884-
/// assert_eq!(b.shape, Col);
920+
/// assert_eq!(b.layout(), Col);
885921
/// ```
886922
fn change_shape(&self) -> Self {
887923
let r = self.row;
@@ -920,9 +956,9 @@ impl MatrixTrait for Matrix {
920956
/// use peroxide::fuga::*;
921957
///
922958
/// let a = matrix(vec![1,2,3,4],2,2,Row);
923-
/// assert_eq!(a.shape, Row);
959+
/// assert_eq!(a.layout(), Row);
924960
/// let b = a.change_shape();
925-
/// assert_eq!(b.shape, Col);
961+
/// assert_eq!(b.layout(), Col);
926962
/// ```
927963
fn change_shape_mut(&mut self) {
928964
let r = self.row;

‎tests/change_shape_test.rs‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ use peroxide::fuga::*;
44
#[test]
55
fn change_shape_test() {
66
let a = matrix(vec![1, 2, 3, 4], 2, 2, Row);
7-
assert_eq!(a.shape, Row);
7+
assert_eq!(a.layout(), Row);
88
let b = a.change_shape();
9-
assert_eq!(b.shape, Col);
9+
assert_eq!(b.layout(), Col);
1010
}

‎tests/complex_matrix.rs‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,18 @@ use peroxide::fuga::*;
66
#[test]
77
fn test_seq() {
88
use num_complex::Complex64;
9-
use peroxide::complex::matrix::ComplexMatrix;
9+
use peroxide::complex::matrix::cmatrix;
1010

11-
let v1 = ComplexMatrix {
12-
data: vec![
11+
let v1 = cmatrix(
12+
vec![
1313
Complex64::new(1f64, 1f64),
1414
Complex64::new(2f64, 2f64),
1515
Complex64::new(3f64, 3f64),
1616
Complex64::new(4f64, 4f64),
1717
],
18-
row: 2,
19-
col: 2,
20-
shape: Row,
21-
};
22-
assert_eq!(v1.data[0], Complex64::new(1f64, 1f64));
18+
2,
19+
2,
20+
Row,
21+
);
22+
assert_eq!(v1.as_slice()[0], Complex64::new(1f64, 1f64));
2323
}

0 commit comments

Comments
 (0)