diff --git a/Cargo.lock b/Cargo.lock index bb2fd7669..85bda3a36 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -7429,10 +7429,13 @@ dependencies = [ "tempfile", "tokio", "toml 0.8.12", + "tracing", "wasm-encoder 0.200.0", + "wasm-metadata 0.200.0", "wasmparser 0.200.0", "wasmtime", "wasmtime-wasi", + "wat", "wit-component 0.200.0", "wit-parser 0.200.0", ] diff --git a/crates/componentize/Cargo.toml b/crates/componentize/Cargo.toml index 229599884..1f62620f1 100644 --- a/crates/componentize/Cargo.toml +++ b/crates/componentize/Cargo.toml @@ -10,8 +10,10 @@ rust-version.workspace = true [dependencies] anyhow = { workspace = true } +tracing = "0.1" wasmparser = "0.200.0" wasm-encoder = "0.200.0" +wasm-metadata = "0.200.0" wit-component = "0.200.0" wit-parser = "0.200.0" @@ -28,3 +30,4 @@ serde = { version = "1.0.197", features = ["derive"] } tempfile = "3.10.0" toml = "0.8.10" serde_json = "1.0" +wat = "1.200.0" \ No newline at end of file diff --git a/crates/componentize/src/bugs.rs b/crates/componentize/src/bugs.rs new file mode 100644 index 000000000..1e7ed9e63 --- /dev/null +++ b/crates/componentize/src/bugs.rs @@ -0,0 +1,118 @@ +use anyhow::bail; +use wasm_metadata::Producers; +use wasmparser::{Encoding, ExternalKind, Parser, Payload}; + +/// Represents the detected likelihood of the allocation bug fixed in +/// https://github.com/WebAssembly/wasi-libc/pull/377 being present in a Wasm +/// module. +#[derive(Debug, PartialEq)] +pub enum WasiLibc377Bug { + ProbablySafe, + ProbablyUnsafe, + Unknown, +} + +impl WasiLibc377Bug { + pub fn detect(module: &[u8]) -> anyhow::Result { + for payload in Parser::new(0).parse_all(module) { + match payload? { + Payload::Version { encoding, .. } if encoding != Encoding::Module => { + bail!("detection only applicable to modules"); + } + Payload::ExportSection(reader) => { + for export in reader { + let export = export?; + if export.kind == ExternalKind::Func && export.name == "cabi_realloc" { + // `cabi_realloc` is a good signal that this module + // uses wit-bindgen, making it probably-safe. + tracing::debug!("Found cabi_realloc export"); + return Ok(Self::ProbablySafe); + } + } + } + Payload::CustomSection(c) if c.name() == "producers" => { + let producers = Producers::from_bytes(c.data(), c.data_offset())?; + if let Some(clang_version) = + producers.get("processed-by").and_then(|f| f.get("clang")) + { + tracing::debug!(clang_version, "Parsed producers.processed-by.clang"); + + // Clang/LLVM version is a good proxy for wasi-sdk + // version; the allocation bug was fixed in wasi-sdk-18 + // and LLVM was updated to 15.0.7 in wasi-sdk-19. + if let Some((major, minor, patch)) = parse_clang_version(clang_version) { + return if (major, minor, patch) >= (15, 0, 7) { + Ok(Self::ProbablySafe) + } else { + Ok(Self::ProbablyUnsafe) + }; + } else { + tracing::warn!( + clang_version, + "Unexpected producers.processed-by.clang version" + ); + } + } + } + _ => (), + } + } + Ok(Self::Unknown) + } +} + +fn parse_clang_version(ver: &str) -> Option<(u16, u16, u16)> { + // Strip optional trailing detail after space + let ver = ver.split(' ').next().unwrap(); + let mut parts = ver.split('.'); + let major = parts.next()?.parse().ok()?; + let minor = parts.next()?.parse().ok()?; + let patch = parts.next()?.parse().ok()?; + Some((major, minor, patch)) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn wasi_libc_377_detect() { + use WasiLibc377Bug::*; + for (wasm, expected) in [ + (r#"(module)"#, Unknown), + ( + r#"(module (func (export "cabi_realloc") (unreachable)))"#, + ProbablySafe, + ), + ( + r#"(module (func (export "some_other_function") (unreachable)))"#, + Unknown, + ), + ( + r#"(module (@producers (processed-by "clang" "16.0.0 extra-stuff")))"#, + ProbablySafe, + ), + ( + r#"(module (@producers (processed-by "clang" "15.0.7")))"#, + ProbablySafe, + ), + ( + r#"(module (@producers (processed-by "clang" "15.0.6")))"#, + ProbablyUnsafe, + ), + ( + r#"(module (@producers (processed-by "clang" "14.0.0")))"#, + ProbablyUnsafe, + ), + ( + r#"(module (@producers (processed-by "clang" "a.b.c")))"#, + Unknown, + ), + ] { + eprintln!("WAT: {wasm}"); + let module = wat::parse_str(wasm).unwrap(); + let detected = WasiLibc377Bug::detect(&module).unwrap(); + assert_eq!(detected, expected); + } + } +} diff --git a/crates/componentize/src/lib.rs b/crates/componentize/src/lib.rs index 987ba7ee7..f4be1bcb1 100644 --- a/crates/componentize/src/lib.rs +++ b/crates/componentize/src/lib.rs @@ -9,6 +9,8 @@ use { wit_component::{metadata, ComponentEncoder}, }; +pub mod bugs; + #[cfg(test)] mod abi_conformance; mod convert; diff --git a/crates/trigger/src/loader.rs b/crates/trigger/src/loader.rs index 70c9d2ba4..7b42fca43 100644 --- a/crates/trigger/src/loader.rs +++ b/crates/trigger/src/loader.rs @@ -1,4 +1,4 @@ -use std::path::PathBuf; +use std::path::{Path, PathBuf}; use anyhow::{ensure, Context, Result}; use async_trait::async_trait; @@ -6,6 +6,7 @@ use spin_app::{ locked::{LockedApp, LockedComponentSource}, AppComponent, Loader, }; +use spin_componentize::bugs::WasiLibc377Bug; use spin_core::StoreBuilder; use tokio::fs; @@ -135,6 +136,7 @@ impl Loader for TriggerLoader { .as_ref() .context("LockedComponentSource missing source field")?; let path = parse_file_url(source)?; + check_uncomponentizable_module_deprecation(&path); spin_core::Module::from_file(engine, &path) .with_context(|| format!("loading module {}", quoted_path(&path))) } @@ -167,6 +169,41 @@ impl Loader for TriggerLoader { } } +// Check whether the given module is (likely) susceptible to a wasi-libc bug +// that makes it unsafe to componentize. If so, print a deprecation warning; +// this will turn into a hard error in a future release. +fn check_uncomponentizable_module_deprecation(module_path: &Path) { + let module = match std::fs::read(module_path) { + Ok(module) => module, + Err(err) => { + tracing::warn!("Failed to read {module_path:?}: {err:#}"); + return; + } + }; + match WasiLibc377Bug::detect(&module) { + Ok(WasiLibc377Bug::ProbablySafe) => {} + not_safe @ Ok(WasiLibc377Bug::ProbablyUnsafe | WasiLibc377Bug::Unknown) => { + println!( + "\n!!! DEPRECATION WARNING !!!\n\ + The Wasm module at {path}\n\ + {verbs} have been compiled with wasi-sdk version <19 and is likely to\n\ + contain a critical memory safety bug. Spin has deprecated execution of these\n\ + modules; they will stop working in a future release.\n\ + For more information, see: https://github.com/fermyon/spin/issues/2552\n", + path = module_path.display(), + verbs = if not_safe.unwrap() == WasiLibc377Bug::ProbablyUnsafe { + "appears to" + } else { + "may" + } + ); + } + Err(err) => { + tracing::warn!("Failed to apply wasi-libc bug heuristic on {module_path:?}: {err:#}"); + } + } +} + #[cfg(test)] mod tests { use super::*; diff --git a/examples/spin-timer/Cargo.lock b/examples/spin-timer/Cargo.lock index 3b9111f36..e647f6d36 100644 --- a/examples/spin-timer/Cargo.lock +++ b/examples/spin-timer/Cargo.lock @@ -5708,7 +5708,9 @@ name = "spin-componentize" version = "2.6.0-pre0" dependencies = [ "anyhow", + "tracing", "wasm-encoder 0.200.0", + "wasm-metadata", "wasmparser 0.200.0", "wit-component", "wit-parser 0.200.0",