Fix some heap type encoding/validation issues (#934)

* Fix some heap type encoding/validation issues

* Be sure to encode the index variant of `HeapType` as a signed integer
  rather than an unsigned integer to have it decoded correctly.
* Additionally add a missing validation on `call_ref` and
  `return_call_ref` to validate the input type before forwarding it to
  other internal match methods.

* Update `check_heap_type`

Author: alexcrichton

crates/wasm-encoder/src/core/types.rs

@@ -112,7 +112,9 @@ impl Encode for HeapType {
         match self {
             HeapType::Func => sink.push(0x70),
             HeapType::Extern => sink.push(0x6F),
-            HeapType::TypedFunc(i) => i.encode(sink),
+            // Note that this is encoded as a signed type rather than unsigned
+            // as it's decoded as an s33
+            HeapType::TypedFunc(i) => i64::from(*i).encode(sink),
         }
     }
 }

crates/wasmparser/src/resources.rs

@@ -14,7 +14,8 @@
  */
 
 use crate::{
-    BinaryReaderError, FuncType, GlobalType, MemoryType, RefType, TableType, ValType, WasmFeatures,
+    BinaryReaderError, FuncType, GlobalType, HeapType, MemoryType, RefType, TableType, ValType,
+    WasmFeatures,
 };
 use std::ops::Range;
 
@@ -228,6 +229,27 @@ pub trait WasmModuleResources {
         offset: usize,
     ) -> Result<(), BinaryReaderError>;
 
+    /// Checks that a `HeapType` is valid, notably its function index if one is
+    /// used.
+    fn check_heap_type(
+        &self,
+        heap_type: HeapType,
+        features: &WasmFeatures,
+        offset: usize,
+    ) -> Result<(), BinaryReaderError> {
+        // Delegate to the generic value type validation which will have the
+        // same validity checks.
+        self.check_value_type(
+            RefType {
+                nullable: true,
+                heap_type,
+            }
+            .into(),
+            features,
+            offset,
+        )
+    }
+
     /// Returns the number of elements.
     fn element_count(&self) -> u32;
     /// Returns the number of bytes in the Wasm data section.

crates/wasmparser/src/validator/operators.rs

@@ -1296,6 +1296,8 @@ where
         Ok(())
     }
     fn visit_call_ref(&mut self, hty: HeapType) -> Self::Output {
+        self.resources
+            .check_heap_type(hty, &self.features, self.offset)?;
         // If `None` is popped then that means a "bottom" type was popped which
         // is always considered equivalent to the `hty` tag.
         if let Some(rt) = self.pop_ref()? {
@@ -2232,15 +2234,8 @@ where
         Ok(())
     }
     fn visit_ref_null(&mut self, heap_type: HeapType) -> Self::Output {
-        self.resources.check_value_type(
-            RefType {
-                nullable: true,
-                heap_type,
-            }
-            .into(),
-            &self.features,
-            self.offset,
-        )?;
+        self.resources
+            .check_heap_type(heap_type, &self.features, self.offset)?;
         self.push_operand(ValType::Ref(RefType {
             nullable: true,
             heap_type,

crates/wast/src/core/binary.rs

@@ -241,8 +241,11 @@ impl<'a> Encode for HeapType<'a> {
             HeapType::Struct => e.push(0x67),
             HeapType::Array => e.push(0x66),
             HeapType::I31 => e.push(0x6a),
-            HeapType::Index(index) => {
-                index.encode(e);
+            // Note that this is encoded as a signed leb128 so be sure to cast
+            // to an i64 first
+            HeapType::Index(Index::Num(n, _)) => i64::from(*n).encode(e),
+            HeapType::Index(Index::Id(n)) => {
+                panic!("unresolved index in emission: {:?}", n)
             }
         }
     }
@@ -427,8 +430,14 @@ impl Encode for Table<'_> {
     fn encode(&self, e: &mut Vec<u8>) {
         assert!(self.exports.names.is_empty());
         match &self.kind {
-            TableKind::Normal { ty, init_expr: None } => ty.encode(e),
-            TableKind::Normal { ty, init_expr: Some(init_expr) } => {
+            TableKind::Normal {
+                ty,
+                init_expr: None,
+            } => ty.encode(e),
+            TableKind::Normal {
+                ty,
+                init_expr: Some(init_expr),
+            } => {
                 e.push(0x40);
                 e.push(0x00);
                 ty.encode(e);

tests/local/function-references/bad-call-ref-ty.wast

@@ -15,3 +15,21 @@
     )
   )
   "expected i32 but nothing on stack")
+
+(assert_invalid
+  (module
+    (type $t (func ))
+    (func (param $f (ref $t))
+      (call_ref 5 (local.get $f))
+    )
+  )
+  "type index out of bounds")
+
+(assert_invalid
+  (module
+    (type $t (func ))
+    (func (param $f (ref $t))
+      (call_ref 100 (local.get $f))
+    )
+  )
+  "type index out of bounds")