Repository: bytecodealliance/wasmtime
Pull request #2506 (2020-12-14 16:05:26)

Fix fuzzer expectation about valid modules
Recent changes to fuzzers made expectations more strict about handling
errors while fuzzing, but this erroneously changed a module compilation
step to always assume that the input wasm is valid. Instead a flag is
now passed through indicating whether the wasm blob is known valid or
invalid, and only if compilation fails and it's known valid do we panic.



Patch:
diff --git a/crates/fuzzing/src/oracles.rs b/crates/fuzzing/src/oracles.rs
index 91e676c58..2d11255f5 100644
--- a/crates/fuzzing/src/oracles.rs
+++ b/crates/fuzzing/src/oracles.rs
@@ -57,8 +57,13 @@ fn log_wat(wat: &str) {
 /// Performs initial validation, and returns early if the Wasm is invalid.
 ///
 /// You can control which compiler is used via passing a `Strategy`.
-pub fn instantiate(wasm: &[u8], strategy: Strategy) {
-    instantiate_with_config(wasm, crate::fuzz_default_config(strategy).unwrap(), None);
+pub fn instantiate(wasm: &[u8], known_valid: bool, strategy: Strategy) {
+    instantiate_with_config(
+        wasm,
+        known_valid,
+        crate::fuzz_default_config(strategy).unwrap(),
+        None,
+    );
 }
 
 /// Instantiate the Wasm buffer, and implicitly fail if we have an unexpected
@@ -67,7 +72,12 @@ pub fn instantiate(wasm: &[u8], strategy: Strategy) {
 /// The engine will be configured using provided config.
 ///
 /// See also `instantiate` functions.
-pub fn instantiate_with_config(wasm: &[u8], mut config: Config, timeout: Option<Duration>) {
+pub fn instantiate_with_config(
+    wasm: &[u8],
+    known_valid: bool,
+    mut config: Config,
+    timeout: Option<Duration>,
+) {
     crate::init_fuzzing();
 
     config.interruptable(timeout.is_some());
@@ -91,7 +101,11 @@ pub fn instantiate_with_config(wasm: &[u8], mut config: Config, timeout: Option<
     }
 
     log_wasm(wasm);
-    let module = Module::new(&engine, wasm).unwrap();
+    let module = match Module::new(&engine, wasm) {
+        Ok(module) => module,
+        Err(_) if !known_valid => return,
+        Err(e) => panic!("failed to compile module: {:?}", e),
+    };
     let imports = dummy_imports(&store, module.imports());
 
     match Instance::new(&store, &module, &imports) {
diff --git a/fuzz/fuzz_targets/instantiate-maybe-invalid.rs b/fuzz/fuzz_targets/instantiate-maybe-invalid.rs
index 5b9657b92..219986a25 100644
--- a/fuzz/fuzz_targets/instantiate-maybe-invalid.rs
+++ b/fuzz/fuzz_targets/instantiate-maybe-invalid.rs
@@ -9,6 +9,7 @@ use wasmtime_fuzzing::oracles;
 fuzz_target!(|module: MaybeInvalidModule| {
     oracles::instantiate_with_config(
         &module.to_bytes(),
+        false,
         wasmtime_fuzzing::fuzz_default_config(Strategy::Auto).unwrap(),
         Some(Duration::from_secs(20)),
     );
diff --git a/fuzz/fuzz_targets/instantiate-swarm.rs b/fuzz/fuzz_targets/instantiate-swarm.rs
index a3049ad99..092a32e92 100644
--- a/fuzz/fuzz_targets/instantiate-swarm.rs
+++ b/fuzz/fuzz_targets/instantiate-swarm.rs
@@ -9,5 +9,5 @@ use wasmtime_fuzzing::oracles;
 fuzz_target!(|module: ConfiguredModule<SwarmConfig>| {
     let mut cfg = wasmtime_fuzzing::fuzz_default_config(Strategy::Auto).unwrap();
     cfg.wasm_multi_memory(true);
-    oracles::instantiate_with_config(&module.to_bytes(), cfg, Some(Duration::from_secs(20)));
+    oracles::instantiate_with_config(&module.to_bytes(), true, cfg, Some(Duration::from_secs(20)));
 });
diff --git a/fuzz/fuzz_targets/instantiate-wasm-smith.rs b/fuzz/fuzz_targets/instantiate-wasm-smith.rs
index 9c081eff8..546a198ff 100644
--- a/fuzz/fuzz_targets/instantiate-wasm-smith.rs
+++ b/fuzz/fuzz_targets/instantiate-wasm-smith.rs
@@ -9,5 +9,5 @@ fuzz_target!(|module: Module| {
     let mut module = module;
     module.ensure_termination(1000);
     let wasm_bytes = module.to_bytes();
-    oracles::instantiate(&wasm_bytes, Strategy::Auto);
+    oracles::instantiate(&wasm_bytes, true, Strategy::Auto);
 });
diff --git a/fuzz/fuzz_targets/instantiate.rs b/fuzz/fuzz_targets/instantiate.rs
index 61b22c187..ec41c5910 100644
--- a/fuzz/fuzz_targets/instantiate.rs
+++ b/fuzz/fuzz_targets/instantiate.rs
@@ -5,5 +5,5 @@ use wasmtime::Strategy;
 use wasmtime_fuzzing::oracles;
 
 fuzz_target!(|data: &[u8]| {
-    oracles::instantiate(data, Strategy::Auto);
+    oracles::instantiate(data, false, Strategy::Auto);
 });


Test patch:
diff --git a/tests/all/fuzzing.rs b/tests/all/fuzzing.rs
index 9e71add57..3e46af2d7 100644
--- a/tests/all/fuzzing.rs
+++ b/tests/all/fuzzing.rs
@@ -11,13 +11,13 @@ use wasmtime_fuzzing::oracles;
 #[test]
 fn instantiate_empty_module() {
     let data = wat::parse_str(include_str!("./fuzzing/empty.wat")).unwrap();
-    oracles::instantiate(&data, Strategy::Auto);
+    oracles::instantiate(&data, true, Strategy::Auto);
 }
 
 #[test]
 fn instantiate_empty_module_with_memory() {
     let data = wat::parse_str(include_str!("./fuzzing/empty_with_memory.wat")).unwrap();
-    oracles::instantiate(&data, Strategy::Auto);
+    oracles::instantiate(&data, true, Strategy::Auto);
 }
 
 #[test]
@@ -26,5 +26,5 @@ fn instantiate_module_that_compiled_to_x64_has_register_32() {
     let mut config = Config::new();
     config.debug_info(true);
     let data = wat::parse_str(include_str!("./fuzzing/issue694.wat")).unwrap();
-    oracles::instantiate_with_config(&data, config, None);
+    oracles::instantiate_with_config(&data, true, config, None);
 }