dimforge / dimforge/nalgebra

Deserialization of `VecStorage` is still unsound

Open
#982 4 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

As it was pointed out in #883, the deserialization code for `Matrix` backed by `VecStorage` was not checking that deserialized data actually has as much elements as dictated by dimensions. That issue was solved by #889 which introduced custom deserialization code which checks that number of elements is equal to product of numeric values of dimensions. However this code did not account for possibility that such a multiplication can overflow. This oversight makes it possible to construct a matrix with huge dimensions but no actual data.

The following is a minimal reproducible example that I managed to come up with:

**Cargo.toml**
```toml
[package]
name = "nalgebra_issue"
version = "0.1.0"

[dependencies]
nalgebra = { version = "0.29", features = ["serde-serialize"] }
serde_json = "1" # I believe it is possible to exploit with other formats as well
```

**src/main.rs**
```rust
// 65536 = 1 << 16
#[cfg(target_pointer_width = "32")]
const JSON: &str = r#"[
[],
65536,
65536
]"#;

// 4294967296 = 1 << 32
#[cfg(target_pointer_width = "64")]
const JSON: &str = r#"[
[],
4294967296,
4294967296
]"#;

fn main() {
let m: nalgebra::base::DMatrix = serde_json::from_str(JSON).unwrap();
println!("{}", m);
}
```

Running this code in debug mode makes it panic with a multiplication overflow message. Running it in release mode, however, segfaults on my machine.

Contributor guide

No contributing guide indexed for this repository

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.