Actyx / Actyx/cambria

Potential Memory Safety Issues

Abierto
#13 0 comentarios 0 reacciones 0 asignados Ver en GitHub
Lenguaje dominante
Idris
Estrellas
38
Forks
2
Métricas de merge de PR
Sin PR fusionados en 30 d

Descripción

Hello,

First, thank you for your work on this interesting crate.

We are developing a static analysis tool for Rust, and during our testing, it flagged a few potential memory safety issues. We were able to confirm them with Miri and wanted to share our findings with you.

The issues are located in src/layout.rs and are related to unsafe blocks that may not have sufficient checks to prevent invalid memory access under certain conditions.

https://github.com/Actyx/cambria/blob/764b9dc7985fc4a2cca4900ce7e3b964cbbb424e/src/layout.rs#L145-L199

### 1: Out-of-Bounds Read in `Ptr::idx`
POC:
```rust
use cambria::{Ptr, Schema};
use rkyv::{archived_root, ser::{serializers::AllocSerializer, Serializer}};
use std::collections::BTreeMap;

fn main() {
let schema = Schema::Array(false, Box::new(Schema::Number));

let mut schema_serializer = AllocSerializer::<256>::default();
schema_serializer.serialize_value(&schema).unwrap();
let schema_bytes = schema_serializer.into_serializer().into_inner();
let archived_schema = unsafe { archived_root::(&schema_bytes) };

let data_val: Vec = vec![1];
let mut data_serializer = AllocSerializer::<256>::default();
data_serializer.serialize_value(&data_val).unwrap();
let data_bytes = data_serializer.into_serializer().into_inner();
let archived_data = unsafe { archived_root::>(&data_bytes) };

let ptr = Ptr::from_ref(archived_data, archived_schema);
let _bad_ptr = ptr.idx(1).unwrap();
}
```
verified with miri:
```
ccuu@ccuu-H3CDesk-D500t:~/Desktop/rust/test1$ cargo +nightly miri run --release
Finished `release` profile [optimized] target(s) in 0.01s
Running `/home/ccuu/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/release/test1`
warning: Miri does not support optimizations: the opt-level is ignored. The only effect of selecting a Cargo profile that enables optimizations (such as --release) is to apply its remaining settings, such as whether debug assertions and overflow checks are enabled.

error: Undefined Behavior: trying to retag from <2910> for SharedReadOnly permission at alloc569[0x0], but that tag does not exist in the borrow stack for this location
--> /home/ccuu/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rkyv-0.7.45/src/boxed.rs:19:18
|
19 | unsafe { &*self.0.as_ptr() }
| ^^^^^^^^^^^^^^^^^ this error occurs as part of retag at alloc569[0x0..0xc]
|
= 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
```

### 2: Integer Underflow in `Ptr::new`
POC:
```rust
fn main() {
let mut fields = BTreeMap::new();
fields.insert("field1".to_string(), Schema::Number);
fields.insert("field2".to_string(), Schema::Number);
let schema = Schema::Object(fields);

let mut schema_serializer = AllocSerializer::<256>::default();
schema_serializer.serialize_value(&schema).unwrap();
let schema_bytes = schema_serializer.into_serializer().into_inner();
let archived_schema = unsafe { archived_root::(&schema_bytes) };

let data_bytes: &[u8] = &[];
let _ptr = Ptr::new(data_bytes, archived_schema);
}
```
verified with miri:
```
ccuu@ccuu-H3CDesk-D500t:~/Desktop/rust/test1$ cargo +nightly miri run --release
Finished `release` profile [optimized] target(s) in 0.02s
Running `/home/ccuu/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/release/test1`
warning: Miri does not support optimizations: the opt-level is ignored. The only effect of selecting a Cargo profile that enables optimizations (such as --release) is to apply its remaining settings, such as whether debug assertions and overflow checks are enabled.

error: Undefined Behavior: overflowing pointer arithmetic: the total offset in bytes does not fit in an `isize`
--> /home/ccuu/Desktop/rust/cambria/src/layout.rs:148:28
|
148 | let ptr = unsafe { (bytes as *const _ as *const u8).add(pos) };
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
```

### 3: Type Confusion in Ptr Accessor Methods
```rust
fn main() {
let large_offset_data: i64 = i64::MAX;

let text_schema = Schema::Text;
let mut schema_serializer = AllocSerializer::<256>::default();
schema_serializer.serialize_value(&text_schema).unwrap();
let schema_bytes = schema_serializer.into_serializer().into_inner();
let archived_schema = unsafe { archived_root::(&schema_bytes) };

let ptr = Ptr::from_ref(&large_offset_data, archived_schema);

let _value = ptr.string();
}

```

verified with miri:
```
ccuu@ccuu-H3CDesk-D500t:~/Desktop/rust/test1$ cargo +nightly miri run --release
Finished `release` profile [optimized] target(s) in 0.02s
Running `/home/ccuu/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/release/test1`
warning: Miri does not support optimizations: the opt-level is ignored. The only effect of selecting a Cargo profile that enables optimizations (such as --release) is to apply its remaining settings, such as whether debug assertions and overflow checks are enabled.

error: Undefined Behavior: pointer not dereferenceable: pointer must be dereferenceable for 127 bytes, but got alloc575 which is only 8 bytes from the end of the allocation
--> /home/ccuu/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rkyv-0.7.45/src/string/repr.rs:107:18
|
107 | unsafe { slice::from_raw_parts(self.as_ptr(), self.len()) }
| ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ Undefined Behavior occurred here
|
= help: this indicates a bug in the program: it performed an invalid operation, and caused Undefined Behavior
= help: see https://doc.rust-lang.org/nightly/reference/behavior-considered-undefined.html for further information
```

### 4: Ptr::get()
POC:
```rust
fn main() {
let mut fields = BTreeMap::new();
fields.insert("b".to_string(), Schema::Number);
fields.insert("a".to_string(), Schema::Text);
let schema = Schema::Object(fields);

let mut data = BTreeMap::new();
data.insert("a".to_string(), "hello".to_string());
data.insert("b".to_string(), "123".to_string());

let mut schema_serializer = AllocSerializer::<256>::default();
schema_serializer.serialize_value(&schema).unwrap();
let schema_bytes = schema_serializer.into_serializer().into_inner();
let archived_schema = unsafe { archived_root::(&schema_bytes) };

let mut data_serializer = AllocSerializer::<256>::default();
data_serializer.serialize_value(&data).unwrap();
let data_bytes = data_serializer.into_serializer().into_inner();
let archived_data = unsafe { archived_root::>(&data_bytes) };

let ptr = Ptr::from_ref(archived_data, archived_schema);

let b_ptr = ptr.get("b").unwrap();

let value = b_ptr.number();
}
```

verified with miri:
```
ccuu@ccuu-H3CDesk-D500t:~/Desktop/rust/test1$ cargo +nightly miri run
Finished `dev` profile [unoptimized + debuginfo] target(s) in 0.02s
Running `/home/ccuu/.rustup/toolchains/nightly-x86_64-unknown-linux-gnu/bin/cargo-miri runner target/miri/x86_64-unknown-linux-gnu/debug/test1`
error: Undefined Behavior: trying to retag from <10176> for SharedReadOnly permission at alloc2481[0x0], but that tag does not exist in the borrow stack for this location
--> /home/ccuu/.cargo/registry/src/index.crates.io-1949cf8c6b5b557f/rkyv-0.7.45/src/collections/btree_map/mod.rs:215:33
|
215 | let root = unsafe { &*self.root.as_ptr() };
| ^^^^^^^^^^^^^^^^^^^^ this error occurs as part of retag at alloc2481[0x0..0xc]
|
= 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
```

Guía de contribución

No hay ninguna guía de contribución indexada para este repositorio

Línea de trabajo

Start with src/layout.rs lines 145-199 and inspect Ptr::idx, Ptr::new, the accessor methods, and Ptr::get against the four supplied Miri reproductions. Run the examples with Miri, trace each unsafe operation, and consider the work complete when the reported cases no longer produce undefined behavior while the pointer APIs retain their intended behavior.

Escrito por el modelo de indexación a partir del texto del issue.

Evaluación

Stack tecnológico
rust
Área
security
Tipo de issue
Error
Dificultad
4/5
Tiempo estimado
3-5 días
Estado de actividad
Estancado
Claridad
Bastante claro
Aptitud para principiantes
35/100

Recibe los nuevos issues en tu correo

Un resumen breve de issues de GitHub para principiantes.