Merge pull request #2571 from lann/uncomponentizable-modules

Add deprecation warning for uncomponentizable modules

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"

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<Self> {
+        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);
+        }
+    }
+}

crates/componentize/src/lib.rs

@@ -9,6 +9,8 @@ use {
     wit_component::{metadata, ComponentEncoder},
 };
 
+pub mod bugs;
+
 #[cfg(test)]
 mod abi_conformance;
 mod convert;

crates/trigger/src/loader.rs

@@ -1,11 +1,12 @@
-use std::path::PathBuf;
+use std::path::{Path, PathBuf};
 
 use anyhow::{ensure, Context, Result};
 use async_trait::async_trait;
 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::*;