From bf41ad844ab0b9584703cda41f3cf5c6ae339c99 Mon Sep 17 00:00:00 2001 From: Niklas Fiekas Date: Wed, 20 Jan 2021 17:47:48 +0100 Subject: [PATCH 1/2] raw-cpuid: Multiple soundness issues --- crates/raw-cpuid/RUSTSEC-0000-0000.md | 59 +++++++++++++++++++++++++++ 1 file changed, 59 insertions(+) create mode 100644 crates/raw-cpuid/RUSTSEC-0000-0000.md diff --git a/crates/raw-cpuid/RUSTSEC-0000-0000.md b/crates/raw-cpuid/RUSTSEC-0000-0000.md new file mode 100644 index 000000000..28d14bb16 --- /dev/null +++ b/crates/raw-cpuid/RUSTSEC-0000-0000.md @@ -0,0 +1,59 @@ +```toml +[advisory] +id = "RUSTSEC-0000-0000" +package = "raw-cpuid" +date = "2021-01-20" +url = "https://github.com/RustSec/advisory-db/pull/614" +categories = ["crash", "memory-corruption"] + +[versions] +patched = [">= TODO"] + +[affected] +arch = ["x86", "x86_64"] +``` + +# Multiple soundness issues in `raw-cpuid` + +## Undefined behavior in `as_string()` methods + +`VendorInfo::as_string()`, `SoCVendorBrand::as_string()`, +and `ExtendedFunctionInfo::processor_brand_string()` construct byte slices +using `std::slice::from_raw_parts()`, with data coming from +`#[repr(Rust)]` structs. This is always undefined behavior. + +See https://github.com/gz/rust-cpuid/issues/40. + +TODO: The flaw has been corrected by making the relevant structs `#[repr(C)]`. + +## Combination of `Deserialize` and `as_string()` is unsound + +The `as_string()` methods then proceed to use +`std::str::from_utf8_unchecked()`, which is usually valid, because real vendor +names etc. are specified to be ASCII. However, if the `serialize` feature is +enabled, it is also possible to construct the structs with arbitrary values +using their `serde::Deserialize` implementation, thus causing undefined +behavior in safe code. + +See https://github.com/gz/rust-cpuid/issues/43. + +## `native_cpuid::cpuid_count()` is technically unsound + +`native_cpuid::cpuid_count()` exposes the unsafe `__cpuid_count()` intrinsic +from `core::arch::x86` or `core::arch::x86_64` as a safe function, without +checking the +[safety requirement](https://doc.rust-lang.org/core/arch/index.html#overview) + +> The CPU the program is currently running on supports the function being +> called. + +which is true for most, but not all, x86/x86_64 CPUs. The crate compiles only +on these architectures, so others are unaffected. + +The function is exposed transitively by the `cpuid!()` macro and used by most +of the crate. + +This flaw is mitigated by the fact that affected programs are expected to crash +deterministically every time. + +See https://github.com/gz/rust-cpuid/issues/41. From 2e01144dc4666d1fbd99737366fb689a83b7fa40 Mon Sep 17 00:00:00 2001 From: Niklas Fiekas Date: Sun, 24 Jan 2021 21:19:04 +0100 Subject: [PATCH 2/2] prepare first part of raw-cpuid advisory, add solutions --- crates/raw-cpuid/RUSTSEC-0000-0000.md | 42 ++++++++++----------------- 1 file changed, 16 insertions(+), 26 deletions(-) diff --git a/crates/raw-cpuid/RUSTSEC-0000-0000.md b/crates/raw-cpuid/RUSTSEC-0000-0000.md index 28d14bb16..bc09fbbd3 100644 --- a/crates/raw-cpuid/RUSTSEC-0000-0000.md +++ b/crates/raw-cpuid/RUSTSEC-0000-0000.md @@ -4,16 +4,16 @@ id = "RUSTSEC-0000-0000" package = "raw-cpuid" date = "2021-01-20" url = "https://github.com/RustSec/advisory-db/pull/614" -categories = ["crash", "memory-corruption"] +categories = ["memory-corruption", "denial-of-service"] [versions] -patched = [">= TODO"] +patched = [">= 9.0.0"] [affected] arch = ["x86", "x86_64"] ``` -# Multiple soundness issues in `raw-cpuid` +# Soundness issues in `raw-cpuid` ## Undefined behavior in `as_string()` methods @@ -24,36 +24,26 @@ using `std::slice::from_raw_parts()`, with data coming from See https://github.com/gz/rust-cpuid/issues/40. -TODO: The flaw has been corrected by making the relevant structs `#[repr(C)]`. +This flaw has been fixed in v9.0.0, by making the relevant structs +`#[repr(C)]`. -## Combination of `Deserialize` and `as_string()` is unsound - -The `as_string()` methods then proceed to use -`std::str::from_utf8_unchecked()`, which is usually valid, because real vendor -names etc. are specified to be ASCII. However, if the `serialize` feature is -enabled, it is also possible to construct the structs with arbitrary values -using their `serde::Deserialize` implementation, thus causing undefined -behavior in safe code. - -See https://github.com/gz/rust-cpuid/issues/43. - -## `native_cpuid::cpuid_count()` is technically unsound +## `native_cpuid::cpuid_count()` is unsound `native_cpuid::cpuid_count()` exposes the unsafe `__cpuid_count()` intrinsic -from `core::arch::x86` or `core::arch::x86_64` as a safe function, without -checking the -[safety requirement](https://doc.rust-lang.org/core/arch/index.html#overview) +from `core::arch::x86` or `core::arch::x86_64` as a safe function, and uses +it internally, without checking the +[safety requirement](https://doc.rust-lang.org/core/arch/index.html#overview): > The CPU the program is currently running on supports the function being > called. -which is true for most, but not all, x86/x86_64 CPUs. The crate compiles only -on these architectures, so others are unaffected. +CPUID is available in most, but not all, x86/x86_64 environments. The crate +compiles only on these architectures, so others are unaffected. -The function is exposed transitively by the `cpuid!()` macro and used by most -of the crate. - -This flaw is mitigated by the fact that affected programs are expected to crash -deterministically every time. +This issue is mitigated by the fact that affected programs are expected +to crash deterministically every time. See https://github.com/gz/rust-cpuid/issues/41. + +The flaw has been fixed in v9.0.0, by intentionally breaking compilation +when targetting SGX or 32-bit x86 without SSE. This covers all affected CPUs.