Fix an accidental regression from #697

crates/wasmparser/src/validator/core.rs

@@ -136,13 +136,7 @@ impl ModuleState {
     ) -> Result<()> {
         self.module
             .check_global_type(&global.ty, features, offset)?;
-        self.check_const_expr(
-            &global.init_expr,
-            global.ty.content_type,
-            features,
-            types,
-            offset,
-        )?;
+        self.check_const_expr(&global.init_expr, global.ty.content_type, features, types)?;
         self.module.assert_mut().globals.push(global.ty);
         Ok(())
     }
@@ -161,7 +155,7 @@ impl ModuleState {
                 offset_expr,
             } => {
                 let ty = self.module.memory_at(memory_index, offset)?.index_type();
-                self.check_const_expr(&offset_expr, ty, features, types, offset)
+                self.check_const_expr(&offset_expr, ty, features, types)
             }
         }
     }
@@ -197,7 +191,7 @@ impl ModuleState {
                     ));
                 }
 
-                self.check_const_expr(&offset_expr, ValType::I32, features, types, offset)?;
+                self.check_const_expr(&offset_expr, ValType::I32, features, types)?;
             }
             ElementKind::Passive | ElementKind::Declared => {
                 if !features.bulk_memory {
@@ -219,7 +213,7 @@ impl ModuleState {
             let offset = items.original_position();
             match items.read()? {
                 ElementItem::Expr(expr) => {
-                    self.check_const_expr(&expr, e.ty, features, types, offset)?;
+                    self.check_const_expr(&expr, e.ty, features, types)?;
                 }
                 ElementItem::Func(f) => {
                     if e.ty != ValType::FuncRef {
@@ -244,7 +238,6 @@ impl ModuleState {
         expected_ty: ValType,
         features: &WasmFeatures,
         types: &TypeList,
-        offset: usize,
     ) -> Result<()> {
         let mut ops = expr.get_operators_reader();
         let mut validator = OperatorValidator::new_const_expr(features, expected_ty);
@@ -333,7 +326,7 @@ impl ModuleState {
                 .visit_operator(offset, &op)?;
         }
 
-        validator.finish(offset)?;
+        validator.finish(ops.original_position())?;
 
         // See comment in `RefFunc` above for why this is an assert.
         assert!(!uninserted_funcref);

crates/wasmparser/src/validator/operators.rs

@@ -69,6 +69,10 @@ pub(crate) struct OperatorValidator {
     control: Vec<Frame>,
     /// The `operands` is the current type stack.
     operands: Vec<Option<ValType>>,
+
+    /// Offset of the `end` instruction which emptied the `control` stack, which
+    /// must be the end of the function.
+    end_which_emptied_control: Option<usize>,
 }
 
 // This structure corresponds to `ctrl_frame` as specified at in the validation
@@ -128,6 +132,7 @@ impl OperatorValidator {
                 height: 0,
                 unreachable: false,
             }],
+            end_which_emptied_control: None,
         };
         let locals = OperatorValidatorTemp {
             inner: &mut ret,
@@ -159,6 +164,7 @@ impl OperatorValidator {
                 height: 0,
                 unreachable: false,
             }],
+            end_which_emptied_control: None,
         }
     }
 
@@ -209,8 +215,20 @@ impl OperatorValidator {
                 "control frames remain at end of function: END opcode expected"
             );
         }
+
+        // The `end` opcode is one byte which means that the `offset` here
+        // should point just beyond the `end` opcode which emptied the control
+        // stack. If not that means more instructions were present after the
+        // control stack was emptied.
+        if offset != self.end_which_emptied_control.unwrap() + 1 {
+            return Err(self.err_beyond_end(offset));
+        }
         Ok(())
     }
+
+    fn err_beyond_end(&self, offset: usize) -> BinaryReaderError {
+        format_op_err!(offset, "operators remaining after end of function")
+    }
 }
 
 impl<R> Deref for OperatorValidatorTemp<'_, '_, R> {
@@ -265,7 +283,10 @@ impl<'resources, R: WasmModuleResources> OperatorValidatorTemp<'_, 'resources, R
     /// expected and a type was successfully popped, but its exact type is
     /// indeterminate because the current block is unreachable.
     fn pop_operand(&mut self, offset: usize, expected: Option<ValType>) -> Result<Option<ValType>> {
-        let control = self.control.last().unwrap();
+        let control = match self.control.last() {
+            Some(c) => c,
+            None => return Err(self.err_beyond_end(offset)),
+        };
         let actual = if self.operands.len() == control.height {
             if control.unreachable {
                 None
@@ -358,7 +379,10 @@ impl<'resources, R: WasmModuleResources> OperatorValidatorTemp<'_, 'resources, R
     fn pop_ctrl(&mut self, offset: usize) -> Result<Frame> {
         // Read the expected type and expected height of the operand stack the
         // end of the frame.
-        let frame = self.control.last().unwrap();
+        let frame = match self.control.last() {
+            Some(f) => f,
+            None => return Err(self.err_beyond_end(offset)),
+        };
         let ty = frame.block_type;
         let height = frame.height;
 
@@ -997,6 +1021,10 @@ where
         for ty in self.results(offset, frame.block_type)? {
             self.push_operand(offset, ty)?;
         }
+
+        if self.control.is_empty() && self.end_which_emptied_control.is_none() {
+            self.end_which_emptied_control = Some(offset);
+        }
         Ok(())
     }
     fn visit_br(&mut self, offset: usize, relative_depth: u32) -> Self::Output {

fuzz/fuzz_targets/validate.rs

@@ -4,6 +4,7 @@ use libfuzzer_sys::*;
 use wasmparser::{Validator, WasmFeatures};
 
 fuzz_target!(|data: &[u8]| {
+    drop(env_logger::try_init());
     let byte1 = match data.get(0) {
         Some(byte) => byte,
         None => return,
@@ -31,5 +32,10 @@ fuzz_target!(|data: &[u8]| {
         sign_extension: (byte2 & 0b1000_0000) != 0,
     });
 
-    drop(validator.validate_all(&data[2..]));
+    let wasm = &data[2..];
+    if log::log_enabled!(log::Level::Debug) {
+        log::debug!("writing input to `test.wasm`");
+        std::fs::write("test.wasm", wasm).unwrap();
+    }
+    drop(validator.validate_all(wasm));
 });

tests/local/invalid/func.wast

@@ -0,0 +1,62 @@
+(assert_invalid
+  (module binary
+    "\00asm" "\01\00\00\00" ;; magic header
+
+    "\01\04"    ;; type section
+    "\01"       ;; 1 count
+    "\60\00\00" ;; no params or results
+
+    "\03\02"    ;; func section
+    "\01"       ;; 1 count
+    "\00"       ;; type 0
+
+    "\0a\05"    ;; code section
+    "\01"       ;; 1 count
+    "\03"       ;; size of function
+    "\00"       ;; no locals
+    "\0b"       ;; end
+    "\01"       ;; nop
+  )
+  "operators remaining after end of function")
+
+(assert_invalid
+  (module binary
+    "\00asm" "\01\00\00\00" ;; magic header
+
+    "\01\04"    ;; type section
+    "\01"       ;; 1 count
+    "\60\00\00" ;; no params or results
+
+    "\03\02"    ;; func section
+    "\01"       ;; 1 count
+    "\00"       ;; type 0
+
+    "\0a\05"    ;; code section
+    "\01"       ;; 1 count
+    "\03"       ;; size of function
+    "\00"       ;; no locals
+    "\0b"       ;; end
+    "\9d"       ;; f64.trunc
+  )
+  "operators remaining after end of function")
+
+(assert_invalid
+  (module binary
+    "\00asm" "\01\00\00\00" ;; magic header
+
+    "\01\04"    ;; type section
+    "\01"       ;; 1 count
+    "\60\00\00" ;; no params or results
+
+    "\03\02"    ;; func section
+    "\01"       ;; 1 count
+    "\00"       ;; type 0
+
+    "\0a\05"    ;; code section
+    "\01"       ;; 1 count
+    "\03"       ;; size of function
+    "\00"       ;; no locals
+    "\0b"       ;; end
+    "\0b"       ;; end
+  )
+  "operators remaining after end of function")