Validate block parameters (bytecodealliance#123)

also fixes read_var_s33()

Author: yurydelendik

src/binary_reader.rs

@@ -535,20 +535,35 @@ impl<'a> BinaryReader<'a> {
     }
 
     pub fn read_var_s33(&mut self) -> Result<i64> {
-        // Note: this is not quite spec compliant, in that it doesn't enforce
-        // that the number is encoded in ceil(N / 7) bytes. We should make a
-        // generic-over-N decoding function and replace all the various
-        // `read_var_{i,s}NN` methods with calls to instantiations of that.
+        // Optimization for single byte.
+        let byte = self.read_u8()?;
+        if (byte & 0x80) == 0 {
+            return Ok(((byte as i8) << 1) as i64 >> 1);
+        }
 
-        let n = self.read_var_i64()?;
-        if n > (1 << 33 - 1) {
-            Err(BinaryReaderError {
-                message: "Invalid var_s33",
-                offset: self.original_position() - 1,
-            })
-        } else {
-            Ok(n)
+        let mut result = (byte & 0x7F) as i64;
+        let mut shift = 7;
+        loop {
+            let byte = self.read_u8()?;
+            result |= ((byte & 0x7F) as i64) << shift;
+            if shift >= 25 {
+                let continuation_bit = (byte & 0x80) != 0;
+                let sign_and_unused_bit = (byte << 1) as i8 >> (33 - shift);
+                if continuation_bit || (sign_and_unused_bit != 0 && sign_and_unused_bit != -1) {
+                    return Err(BinaryReaderError {
+                        message: "Invalid var_i33",
+                        offset: self.original_position() - 1,
+                    });
+                }
+                return Ok(result);
+            }
+            shift += 7;
+            if (byte & 0x80) == 0 {
+                break;
+            }
         }
+        let ashift = 64 - shift;
+        Ok((result << ashift) >> ashift)
     }
 
     pub fn read_var_i64(&mut self) -> Result<i64> {

src/operators_validator.rs

@@ -550,10 +550,7 @@ impl OperatorValidator {
                     return Err("type index out of bounds");
                 }
                 let ty = &types[idx];
-                if self.config.enable_multi_value {
-                    // TODO implement proper validation that includes params
-                    // similar to `self.check_operands(&ty.params)?;`
-                } else {
+                if !self.config.enable_multi_value {
                     if ty.returns.len() > 1 {
                         return Err("blocks, loops, and ifs may only return at most one \
                                     value when multi-value is not enabled");
@@ -569,6 +566,28 @@ impl OperatorValidator {
         }
     }
 
+    fn check_block_params(
+        &self,
+        ty: TypeOrFuncType,
+        resources: &dyn WasmModuleResources,
+        skip: usize,
+    ) -> OperatorValidatorResult<()> {
+        if let TypeOrFuncType::FuncType(idx) = ty {
+            let func_ty = &resources.types()[idx as usize];
+            let len = func_ty.params.len();
+            self.check_frame_size(len + skip)?;
+            for i in 0..len {
+                if !self
+                    .func_state
+                    .assert_stack_type_at(len - 1 - i + skip, func_ty.params[i])
+                {
+                    return Err("stack operand type mismatch for block");
+                }
+            }
+        }
+        Ok(())
+    }
+
     fn check_select(&self) -> OperatorValidatorResult<Option<Type>> {
         self.check_frame_size(3)?;
         let func_state = &self.func_state;
@@ -610,16 +629,19 @@ impl OperatorValidator {
             Operator::Nop => (),
             Operator::Block { ty } => {
                 self.check_block_type(ty, resources)?;
+                self.check_block_params(ty, resources, 0)?;
                 self.func_state
                     .push_block(ty, BlockType::Block, resources)?;
             }
             Operator::Loop { ty } => {
                 self.check_block_type(ty, resources)?;
+                self.check_block_params(ty, resources, 0)?;
                 self.func_state.push_block(ty, BlockType::Loop, resources)?;
             }
             Operator::If { ty } => {
                 self.check_block_type(ty, resources)?;
                 self.check_operands_1(Type::I32)?;
+                self.check_block_params(ty, resources, 1)?;
                 self.func_state.push_block(ty, BlockType::If, resources)?;
             }
             Operator::Else => {