Account for invalid names in roundtrip fuzzing (#237)

This fixes a bug in the roundtrip fuzzing target where an input `*.wat`
file can produce a wasm file with an "invalid" name section because the
name might be too large. This means that the roundtrip-ing of that file
would not preserve the name section, causing the previous assertion to
fail.

The fuzzer now validates that the name section, if present, is valid
before proceeding with fuzzing.

Author: alexcrichton

crates/wasmparser/src/binary_reader.rs

@@ -680,7 +680,7 @@ impl<'a> BinaryReader<'a> {
         let len = self.read_var_u32()? as usize;
         if len > MAX_WASM_STRING_SIZE {
             return Err(BinaryReaderError::new(
-                "string size in out of bounds",
+                "string size out of bounds",
                 self.original_position() - 1,
             ));
         }
@@ -829,7 +829,7 @@ impl<'a> BinaryReader<'a> {
         let len = self.read_var_u32()? as usize;
         if len > MAX_WASM_STRING_SIZE {
             return Err(BinaryReaderError::new(
-                "string size in out of bounds",
+                "string size out of bounds",
                 self.original_position() - 1,
             ));
         }

fuzz/fuzz_targets/roundtrip.rs

@@ -24,12 +24,73 @@ fuzz_target!(|data: &[u8]| {
         Ok(bytes) => bytes,
         Err(_) => return,
     };
+
+    // Only roundtrip valid modules for now since invalid modules can often have
+    // bizarre structures which aren't intended to print correctly or roundtrip
+    // well.
     if wasmparser::validate(&wasm).is_err() {
         return;
     }
-    let string = match wasmprinter::print_bytes(&wasm) {
+
+    // And finally validate that the name section, if present, is valid. This
+    // can be invalid if names in the name section are too long (e.g. exceeding
+    // the maximum length of a string). The printing process will skip invalid
+    // name sections, so if it's invalid then our roundtrip'd bytes will
+    // trivially not match, but not in an interesting way.
+    if validate_name_section(&wasm).is_err() {
+        return;
+    }
+    let string2 = match wasmprinter::print_bytes(&wasm) {
         Ok(s) => s,
         Err(_) => return,
     };
-    assert_eq!(wasm, wat::parse_str(&string).unwrap());
+
+    let wasm2 = wat::parse_str(&string2).unwrap();
+    if wasm == wasm2 {
+        return;
+    }
+
+    std::fs::write("wasm1.wasm", &wasm).unwrap();
+    std::fs::write("wasm1.wat", &string).unwrap();
+    std::fs::write("wasm2.wasm", &wasm2).unwrap();
+    std::fs::write("wasm2.wat", &string2).unwrap();
+    panic!("wasm bytes differ on roundtrip");
 });
+
+fn validate_name_section(wasm: &[u8]) -> wasmparser::Result<()> {
+    use wasmparser::*;
+    for payload in Parser::new(0).parse_all(wasm) {
+        let reader = match payload? {
+            Payload::CustomSection {
+                name: "name",
+                data_offset,
+                data,
+            } => NameSectionReader::new(data, data_offset)?,
+            _ => continue,
+        };
+        for section in reader {
+            match section? {
+                Name::Module(n) => {
+                    n.get_name()?;
+                }
+                Name::Function(n) => {
+                    let mut map = n.get_map()?;
+                    for _ in 0..map.get_count() {
+                        map.read()?;
+                    }
+                }
+                Name::Local(n) => {
+                    let mut reader = n.get_function_local_reader()?;
+                    for _ in 0..reader.get_count() {
+                        let local_name = reader.read()?;
+                        let mut map = local_name.get_map()?;
+                        for _ in 0..map.get_count() {
+                            map.read()?;
+                        }
+                    }
+                }
+            }
+        }
+    }
+    Ok(())
+}