Move precompiled module detection into wasmtime (#5342)

* Treat `-` as an alias to `/dev/stdin`

This applies to unix targets only,
as Windows does not have an appropriate alternative.

* Add tests for piped modules from stdin

This applies to unix targets only,
as Windows does not have an appropriate alternative.

* Move precompiled module detection into wasmtime

Previously, wasmtime-cli checked the module to be loaded is
precompiled or not, by pre-opening the given file path to
check if the "\x7FELF" header exists.
This commit moves this branch into the `Module::from_trusted_file`,
which is only invoked with `--allow-precompiled` flag on CLI.

The initial motivation of the commit is, feeding a module to wasmtime
from piped inputs, is blocked by the pre-opening of the module.
The `Module::from_trusted_file`, assumes the --allow-precompiled flag
so there is no piped inputs, happily mmap-ing the module to test
if the header exists.
If --allow-precompiled is not supplied, the existing `Module::from_file`
will be used, without the additional header check as the precompiled
modules are intentionally not allowed on piped inputs for security measures.

One caveat of this approach is that the user may be confused if
he or she tries to execute a precompiled module without
--allow-precompiled, as wasmtime shows an 'input bytes aren't valid
utf-8' error, not directly getting what's going wrong.
So this commit includes a hack-ish workaround for this.

Thanks to @jameysharp for suggesting this idea with a detailed guidance.

crates/wasmtime/src/engine.rs

@@ -600,7 +600,7 @@ impl Engine {
         self.load_code(MmapVec::from_slice(bytes)?, expected)
     }
 
-    /// Like `load_code_bytes`, but crates a mmap from a file on disk.
+    /// Like `load_code_bytes`, but creates a mmap from a file on disk.
     pub(crate) fn load_code_file(
         &self,
         path: &Path,
@@ -614,7 +614,7 @@ impl Engine {
         )
     }
 
-    fn load_code(&self, mmap: MmapVec, expected: ObjectKind) -> Result<Arc<CodeMemory>> {
+    pub(crate) fn load_code(&self, mmap: MmapVec, expected: ObjectKind) -> Result<Arc<CodeMemory>> {
         serialization::check_compatible(self, &mmap, expected)?;
         let mut code = CodeMemory::new(mmap)?;
         code.publish()?;

crates/wasmtime/src/module.rs

@@ -332,8 +332,41 @@ impl Module {
         }
     }
 
-    /// Compiles a binary-encoded WebAssembly module to an artifact usable by
+    /// Creates a new WebAssembly `Module` from the contents of the given `file`
-    /// Wasmtime.
+    /// on disk, but with assumptions that the file is from a trusted source.
+    /// The file should be a binary- or text-format WebAssembly module, or a
+    /// precompiled artifact generated by the same version of Wasmtime.
+    ///
+    /// # Unsafety
+    ///
+    /// All of the reasons that [`deserialize`] is `unsafe` apply to this
+    /// function as well. Arbitrary data loaded from a file may trick Wasmtime
+    /// into arbitrary code execution since the contents of the file are not
+    /// validated to be a valid precompiled module.
+    ///
+    /// [`deserialize`]: Module::deserialize
+    ///
+    /// Additionally though this function is also `unsafe` because the file
+    /// referenced must remain unchanged and a valid precompiled module for the
+    /// entire lifetime of the [`Module`] returned. Any changes to the file on
+    /// disk may change future instantiations of the module to be incorrect.
+    /// This is because the file is mapped into memory and lazily loaded pages
+    /// reflect the current state of the file, not necessarily the origianl
+    /// state of the file.
+    #[cfg(compiler)]
+    #[cfg_attr(nightlydoc, doc(cfg(feature = "cranelift")))] // see build.rs
+    pub unsafe fn from_trusted_file(engine: &Engine, file: impl AsRef<Path>) -> Result<Module> {
+        let mmap = MmapVec::from_file(file.as_ref())?;
+        if &mmap[0..4] == b"\x7fELF" {
+            let code = engine.load_code(mmap, ObjectKind::Module)?;
+            return Module::from_parts(engine, code, None);
+        }
+
+        Module::new(engine, &*mmap)
+    }
+
+    /// Converts an input binary-encoded WebAssembly module to compilation
+    /// artifacts and type information.
     ///
     /// This is where compilation actually happens of WebAssembly modules and
     /// translation/parsing/validation of the binary input occurs. The binary

src/commands/run.rs

@@ -3,8 +3,6 @@
 use anyhow::{anyhow, bail, Context as _, Result};
 use clap::Parser;
 use once_cell::sync::Lazy;
-use std::fs::File;
-use std::io::Read;
 use std::thread;
 use std::time::Duration;
 use std::{
@@ -29,6 +27,8 @@ fn parse_module(s: &OsStr) -> anyhow::Result<PathBuf> {
         Some("help") | Some("config") | Some("run") | Some("wast") | Some("compile") => {
             bail!("module name cannot be the same as a subcommand")
         }
+        #[cfg(unix)]
+        Some("-") => Ok(PathBuf::from("/dev/stdin")),
         _ => Ok(s.into()),
     }
 }
@@ -423,27 +423,12 @@ impl RunCommand {
     }
 
     fn load_module(&self, engine: &Engine, path: &Path) -> Result<Module> {
-        // Peek at the first few bytes of the file to figure out if this is
+        if self.allow_precompiled {
-        // something we can pass off to `deserialize_file` which is fastest if
+            unsafe { Module::from_trusted_file(engine, path) }
-        // we don't actually read the whole file into memory. Note that this
+        } else {
-        // behavior is disabled by default, though, because it's not safe to
+            Module::from_file(engine, path)
-        // pass arbitrary user input to this command with `--allow-precompiled`
+                .context("if you're trying to run a precompiled module, pass --allow-precompiled")
-        let mut file =
-            File::open(path).with_context(|| format!("failed to open: {}", path.display()))?;
-        let mut magic = [0; 4];
-        if let Ok(()) = file.read_exact(&mut magic) {
-            if &magic == b"\x7fELF" {
-                if self.allow_precompiled {
-                    return unsafe { Module::deserialize_file(engine, path) };
-                }
-                bail!(
-                    "cannot load precompiled module `{}` unless --allow-precompiled is passed",
-                    path.display()
-                )
-            }
         }
-
-        Module::from_file(engine, path)
     }
 }
 

tests/all/cli_tests.rs

@@ -1,11 +1,13 @@
-use anyhow::{bail, Result};
+use anyhow::{bail, Context, Result};
-use std::io::Write;
+use std::fs::File;
+use std::io::{Read, Write};
 use std::path::Path;
-use std::process::{Command, Output};
+use std::process::{Command, Output, Stdio};
 use tempfile::{NamedTempFile, TempDir};
 
 // Run the wasmtime CLI with the provided args and return the `Output`.
-fn run_wasmtime_for_output(args: &[&str]) -> Result<Output> {
+// If the `stdin` is `Some`, opens the file and redirects to the child's stdin.
+fn run_wasmtime_for_output(args: &[&str], stdin: Option<&Path>) -> Result<Output> {
     let runner = std::env::vars()
         .filter(|(k, _v)| k.starts_with("CARGO_TARGET") && k.ends_with("RUNNER"))
         .next();
@@ -14,6 +16,11 @@ fn run_wasmtime_for_output(args: &[&str]) -> Result<Output> {
     me.pop(); // chop off `deps`
     me.push("wasmtime");
 
+    let stdin = stdin
+        .map(File::open)
+        .transpose()
+        .context("Cannot open a file to use as stdin")?;
+
     // If we're running tests with a "runner" then we might be doing something
     // like cross-emulation, so spin up the emulator rather than the tests
     // itself, which may not be natively executable.
@@ -28,13 +35,33 @@ fn run_wasmtime_for_output(args: &[&str]) -> Result<Output> {
     } else {
         Command::new(&me)
     };
-    cmd.args(args).output().map_err(Into::into)
+
+    if let Some(mut f) = stdin {
+        let mut buf = Vec::new();
+        f.read_to_end(&mut buf)?;
+
+        let mut child = cmd
+            .stdout(Stdio::piped())
+            .stdin(Stdio::piped())
+            .args(args)
+            .spawn()?;
+
+        let mut stdin = child.stdin.take().unwrap();
+        std::thread::spawn(move || {
+            stdin
+                .write_all(&buf)
+                .expect("failed to write module to child stdin")
+        });
+        child.wait_with_output().map_err(Into::into)
+    } else {
+        cmd.args(args).output().map_err(Into::into)
+    }
 }
 
 // Run the wasmtime CLI with the provided args and, if it succeeds, return
 // the standard output in a `String`.
 fn run_wasmtime(args: &[&str]) -> Result<String> {
-    let output = run_wasmtime_for_output(args)?;
+    let output = run_wasmtime_for_output(args, None)?;
     if !output.status.success() {
         bail!(
             "Failed to execute wasmtime with: {:?}\n{}",
@@ -124,7 +151,8 @@ fn run_wasmtime_simple_wat() -> Result<()> {
 #[test]
 fn run_wasmtime_unreachable_wat() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/unreachable.wat")?;
-    let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?;
+    let output =
+        run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?;
 
     assert_ne!(output.stderr, b"");
     assert_eq!(output.stdout, b"");
@@ -164,13 +192,16 @@ fn hello_wasi_snapshot1() -> Result<()> {
 #[test]
 fn timeout_in_start() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/iloop-start.wat")?;
-    let output = run_wasmtime_for_output(&[
+    let output = run_wasmtime_for_output(
-        "run",
+        &[
-        wasm.path().to_str().unwrap(),
+            "run",
-        "--wasm-timeout",
+            wasm.path().to_str().unwrap(),
-        "1ms",
+            "--wasm-timeout",
-        "--disable-cache",
+            "1ms",
-    ])?;
+            "--disable-cache",
+        ],
+        None,
+    )?;
     assert!(!output.status.success());
     assert_eq!(output.stdout, b"");
     let stderr = String::from_utf8_lossy(&output.stderr);
@@ -185,13 +216,16 @@ fn timeout_in_start() -> Result<()> {
 #[test]
 fn timeout_in_invoke() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/iloop-invoke.wat")?;
-    let output = run_wasmtime_for_output(&[
+    let output = run_wasmtime_for_output(
-        "run",
+        &[
-        wasm.path().to_str().unwrap(),
+            "run",
-        "--wasm-timeout",
+            wasm.path().to_str().unwrap(),
-        "1ms",
+            "--wasm-timeout",
-        "--disable-cache",
+            "1ms",
-    ])?;
+            "--disable-cache",
+        ],
+        None,
+    )?;
     assert!(!output.status.success());
     assert_eq!(output.stdout, b"");
     let stderr = String::from_utf8_lossy(&output.stderr);
@@ -207,7 +241,8 @@ fn timeout_in_invoke() -> Result<()> {
 #[test]
 fn exit2_wasi_snapshot0() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit2_wasi_snapshot0.wat")?;
-    let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?;
+    let output =
+        run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?;
     assert_eq!(output.status.code().unwrap(), 2);
     Ok(())
 }
@@ -216,7 +251,8 @@ fn exit2_wasi_snapshot0() -> Result<()> {
 #[test]
 fn exit2_wasi_snapshot1() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit2_wasi_snapshot1.wat")?;
-    let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?;
+    let output =
+        run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?;
     assert_eq!(output.status.code().unwrap(), 2);
     Ok(())
 }
@@ -225,7 +261,8 @@ fn exit2_wasi_snapshot1() -> Result<()> {
 #[test]
 fn exit125_wasi_snapshot0() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit125_wasi_snapshot0.wat")?;
-    let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?;
+    let output =
+        run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?;
     if cfg!(windows) {
         assert_eq!(output.status.code().unwrap(), 1);
     } else {
@@ -238,7 +275,8 @@ fn exit125_wasi_snapshot0() -> Result<()> {
 #[test]
 fn exit125_wasi_snapshot1() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit125_wasi_snapshot1.wat")?;
-    let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?;
+    let output =
+        run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?;
     if cfg!(windows) {
         assert_eq!(output.status.code().unwrap(), 1);
     } else {
@@ -251,7 +289,8 @@ fn exit125_wasi_snapshot1() -> Result<()> {
 #[test]
 fn exit126_wasi_snapshot0() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit126_wasi_snapshot0.wat")?;
-    let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?;
+    let output =
+        run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?;
     assert_eq!(output.status.code().unwrap(), 1);
     assert!(output.stdout.is_empty());
     assert!(String::from_utf8_lossy(&output.stderr).contains("invalid exit status"));
@@ -262,7 +301,8 @@ fn exit126_wasi_snapshot0() -> Result<()> {
 #[test]
 fn exit126_wasi_snapshot1() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit126_wasi_snapshot1.wat")?;
-    let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?;
+    let output =
+        run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?;
     assert_eq!(output.status.code().unwrap(), 1);
     assert!(output.stdout.is_empty());
     assert!(String::from_utf8_lossy(&output.stderr).contains("invalid exit status"));
@@ -368,7 +408,8 @@ fn greeter_preload_callable_command() -> Result<()> {
 #[test]
 fn exit_with_saved_fprs() -> Result<()> {
     let wasm = build_wasm("tests/all/cli_tests/exit_with_saved_fprs.wat")?;
-    let output = run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"])?;
+    let output =
+        run_wasmtime_for_output(&[wasm.path().to_str().unwrap(), "--disable-cache"], None)?;
     assert_eq!(output.status.code().unwrap(), 0);
     assert!(output.stdout.is_empty());
     Ok(())
@@ -389,3 +430,46 @@ fn run_cwasm() -> Result<()> {
     assert_eq!(stdout, "");
     Ok(())
 }
+
+#[cfg(unix)]
+#[test]
+fn hello_wasi_snapshot0_from_stdin() -> Result<()> {
+    // Run a simple WASI hello world, snapshot0 edition.
+    // The module is piped from standard input.
+    let wasm = build_wasm("tests/all/cli_tests/hello_wasi_snapshot0.wat")?;
+    let stdout = {
+        let path = wasm.path();
+        let args: &[&str] = &["-", "--disable-cache"];
+        let output = run_wasmtime_for_output(args, Some(path))?;
+        if !output.status.success() {
+            bail!(
+                "Failed to execute wasmtime with: {:?}\n{}",
+                args,
+                String::from_utf8_lossy(&output.stderr)
+            );
+        }
+        Ok::<_, anyhow::Error>(String::from_utf8(output.stdout).unwrap())
+    }?;
+    assert_eq!(stdout, "Hello, world!\n");
+    Ok(())
+}
+
+#[cfg(unix)]
+#[test]
+fn run_cwasm_from_stdin() -> Result<()> {
+    let td = TempDir::new()?;
+    let cwasm = td.path().join("foo.cwasm");
+    let stdout = run_wasmtime(&[
+        "compile",
+        "tests/all/cli_tests/simple.wat",
+        "-o",
+        cwasm.to_str().unwrap(),
+    ])?;
+    assert_eq!(stdout, "");
+    let args: &[&str] = &["run", "--allow-precompiled", "-"];
+    let output = run_wasmtime_for_output(args, Some(&cwasm))?;
+    if output.status.success() {
+        bail!("wasmtime should fail loading precompiled modules from piped files, but suceeded");
+    }
+    Ok(())
+}