Always use fallible accesses of the GC heap (#13320)

This commit is an attempt to harden Wasmtime in the face of GC heap
corruption to downgrade panics to an error being returned instead.
Normal operation should never hit any of these paths and in theory this
is all dead code. The intention, however, is to further downgrade the
severity of GC heaps from a DoS to, in theory, maybe not even a CVE at
all.

This commit is inspired by the transition done for component-model-async
recently too where many `assert!`'d conditions and panics were
translated into `bail_bug!` within Wasmtime. This returns a special kind
of error in release mode and panics in debug mode. The rationale behind
this is that, like component-model-async, the GC implementation is the
intersection of:

* Easy for guests to control.
* Difficult to guarantee 100% correctness of the host.
* Low consequences if corruption is detected.
* Easy to generate a trap via `?` to propagate upwards.

In this situation the goal here is to more aggressively return errors,
in release mode, rather than panic which risks a quick DoS of
embedders. The ideal goal is for GC heap corruption to not be a DoS at
all, but we're not quite ready to make that commitment just yet.

Many methods in this commit were refactored to return `Result`, and many
implementations internally within the GC implementation have been
updated to use `bail_bug!` or similar to downgrade panics to errors.
Note that in debug mode (or `cfg(debug_assertions)`) all of these are
still panics.

cc #13216

Author: alexcrichton

crates/core/src/lib.rs

@@ -24,4 +24,5 @@ pub mod math;
 pub mod mpk;
 pub mod non_max;
 pub mod slab;
+pub mod truncate;
 pub mod undo;

crates/core/src/truncate.rs

@@ -0,0 +1,11 @@
+//! Explicit methods to clearly indicate that truncation is desired when used.
+
+/// Explicitly truncate an `i32` to an `i16`.
+pub fn truncate_i32_to_i16(a: i32) -> i16 {
+    a as i16
+}
+
+/// Explicitly truncate an `i32` to an `i8`.
+pub fn truncate_i32_to_i8(a: i32) -> i8 {
+    a as i8
+}

crates/environ/src/builtin.rs

@@ -391,10 +391,12 @@ impl BuiltinFunctionIndex {
             // The final epoch represents a trap
             (@get new_epoch u64) => (TrapSentinel::NegativeOne);
 
+            // Failure here indicates GC heap corruption.
+            (@get get_interned_func_ref pointer) => (TrapSentinel::NegativeOne);
+
             // These libcalls can't trap
             (@get ref_func pointer) => (return None);
             (@get table_get_lazy_init_func_ref pointer) => (return None);
-            (@get get_interned_func_ref pointer) => (return None);
             (@get intern_func_ref_for_gc_heap u64) => (return None);
             (@get is_subtype u32) => (return None);
             (@get ceil_f32 f32) => (return None);

crates/wasmtime/src/runtime/externals/global.rs

@@ -270,27 +270,27 @@ impl Global {
                         Some(e) => Some(e.try_gc_ref(&store)?.unchecked_copy()),
                     };
                     let new = new.as_ref();
-                    definition.write_gc_ref(&mut store, new);
+                    definition.write_gc_ref(&mut store, new)?;
                 }
                 Val::AnyRef(a) => {
                     let new = match a {
                         None => None,
                         Some(a) => Some(a.try_gc_ref(&store)?.unchecked_copy()),
                     };
                     let new = new.as_ref();
-                    definition.write_gc_ref(&mut store, new);
+                    definition.write_gc_ref(&mut store, new)?;
                 }
                 Val::ExnRef(e) => {
                     let new = match e {
                         None => None,
                         Some(e) => Some(e.try_gc_ref(&store)?.unchecked_copy()),
                     };
                     let new = new.as_ref();
-                    definition.write_gc_ref(&mut store, new);
+                    definition.write_gc_ref(&mut store, new)?;
                 }
                 Val::ContRef(None) => {
                     // Allow null continuation references for globals - these are just placeholders
-                    definition.write_gc_ref(&mut store, None);
+                    definition.write_gc_ref(&mut store, None)?;
                 }
                 Val::ContRef(Some(_)) => {
                     // TODO(#10248): Implement non-null global continuation reference handling

crates/wasmtime/src/runtime/func.rs

@@ -2157,11 +2157,11 @@ impl<T> Caller<'_, T> {
     ///
     /// Same as [`Store::gc_async`](crate::Store::gc_async).
     #[cfg(all(feature = "async", feature = "gc"))]
-    pub async fn gc_async(&mut self, why: Option<&crate::GcHeapOutOfMemory<()>>)
+    pub async fn gc_async(&mut self, why: Option<&crate::GcHeapOutOfMemory<()>>) -> Result<()>
     where
         T: Send + 'static,
     {
-        self.store.gc_async(why).await;
+        self.store.gc_async(why).await
     }
 
     /// Returns the remaining fuel in the store.

crates/wasmtime/src/runtime/gc/disabled/rooting.rs

@@ -118,6 +118,10 @@ impl<T: GcRef> Rooted<T> {
     pub(crate) fn try_gc_ref<'a>(&self, _store: &'a StoreOpaque) -> Result<&'a VMGcRef> {
         match self.inner {}
     }
+
+    pub(crate) fn try_clone_gc_ref(&self, _: &mut AutoAssertNoGc<'_>) -> Result<VMGcRef> {
+        match self.inner {}
+    }
 }
 
 /// This type has been disabled because the `gc` cargo feature was not enabled

crates/wasmtime/src/runtime/gc/enabled/anyref.rs

@@ -1,11 +1,11 @@
 //! Implementation of `anyref` in Wasmtime.
 
-use super::{ExternRef, RootedGcRefImpl};
+use super::ExternRef;
 use crate::prelude::*;
 use crate::runtime::vm::VMGcRef;
 use crate::{
     ArrayRef, ArrayType, AsContext, AsContextMut, EqRef, GcRefImpl, GcRootIndex, HeapType, I31,
-    OwnedRooted, RefType, Result, Rooted, StructRef, StructType, ValRaw, ValType, WasmTy,
+    OwnedRooted, RefType, Result, Rooted, StructRef, StructType, ValRaw, ValType, WasmTy, bail_bug,
     store::{AutoAssertNoGc, StoreOpaque},
 };
 use core::mem;
@@ -298,11 +298,13 @@ impl AnyRef {
                 || store
                     .unwrap_gc_store()
                     .header(&gc_ref)
+                    .unwrap()
                     .kind()
                     .matches(VMGcKind::AnyRef)
                 || store
                     .unwrap_gc_store()
                     .header(&gc_ref)
+                    .unwrap()
                     .kind()
                     .matches(VMGcKind::ExternRef)
         );
@@ -336,7 +338,9 @@ impl AnyRef {
         let raw = if gc_ref.is_i31() {
             gc_ref.as_raw_non_zero_u32()
         } else {
-            store.require_gc_store_mut()?.expose_gc_ref_to_wasm(gc_ref)
+            store
+                .require_gc_store_mut()?
+                .expose_gc_ref_to_wasm(gc_ref)?
         };
         Ok(raw.get())
     }
@@ -360,29 +364,33 @@ impl AnyRef {
             return Ok(HeapType::I31);
         }
 
-        let header = store.require_gc_store()?.header(gc_ref);
+        let header = store.require_gc_store()?.header(gc_ref)?;
 
         if header.kind().matches(VMGcKind::ExternRef) {
             return Ok(HeapType::Any);
         }
 
         debug_assert!(header.kind().matches(VMGcKind::AnyRef));
         debug_assert!(header.kind().matches(VMGcKind::EqRef));
+        let ty = match header.ty() {
+            Some(ty) => ty,
+            None => bail_bug!("ty should be present"),
+        };
 
         if header.kind().matches(VMGcKind::StructRef) {
             return Ok(HeapType::ConcreteStruct(
-                StructType::from_shared_type_index(store.engine(), header.ty().unwrap()),
+                StructType::from_shared_type_index(store.engine(), ty),
             ));
         }
 
         if header.kind().matches(VMGcKind::ArrayRef) {
             return Ok(HeapType::ConcreteArray(ArrayType::from_shared_type_index(
                 store.engine(),
-                header.ty().unwrap(),
+                ty,
             )));
         }
 
-        unreachable!("no other kinds of `anyref`s")
+        bail_bug!("no other kinds of `anyref`s")
     }
 
     /// Does this `anyref` match the given type?
@@ -436,7 +444,7 @@ impl AnyRef {
         Ok(gc_ref.is_i31()
             || store
                 .require_gc_store()?
-                .kind(gc_ref)
+                .kind(gc_ref)?
                 .matches(VMGcKind::EqRef))
     }
 
@@ -564,7 +572,7 @@ impl AnyRef {
         Ok(!gc_ref.is_i31()
             && store
                 .require_gc_store()?
-                .kind(gc_ref)
+                .kind(gc_ref)?
                 .matches(VMGcKind::StructRef))
     }
 
@@ -632,7 +640,7 @@ impl AnyRef {
         Ok(!gc_ref.is_i31()
             && store
                 .require_gc_store()?
-                .kind(gc_ref)
+                .kind(gc_ref)?
                 .matches(VMGcKind::ArrayRef))
     }
 

crates/wasmtime/src/runtime/gc/enabled/arrayref.rs

@@ -393,7 +393,7 @@ impl ArrayRef {
             "attempted to use a `ArrayRefPre` with the wrong store"
         );
 
-        let len = u32::try_from(elems.len()).unwrap();
+        let len = u32::try_from(elems.len())?;
 
         // Allocate the array.
         let arrayref = store
@@ -416,15 +416,17 @@ impl ArrayRef {
         match (|| {
             let elem_ty = allocator.ty.element_type();
             for (i, elem) in elems.enumerate() {
-                let i = u32::try_from(i).unwrap();
+                let i = u32::try_from(i)?;
                 debug_assert!(i < len);
                 arrayref.initialize_elem(&mut store, allocator.layout(), &elem_ty, i, *elem)?;
             }
             Ok(())
         })() {
             Ok(()) => Ok(Rooted::new(&mut store, arrayref.into())),
             Err(e) => {
-                store.require_gc_store_mut()?.dealloc_uninit_array(arrayref);
+                store
+                    .require_gc_store_mut()?
+                    .dealloc_uninit_array(arrayref)?;
                 Err(e)
             }
         }
@@ -613,11 +615,11 @@ impl ArrayRef {
         assert!(self.comes_from_same_store(store));
         let gc_ref = self.inner.try_gc_ref(store)?;
         debug_assert!({
-            let header = store.require_gc_store()?.header(gc_ref);
+            let header = store.require_gc_store()?.header(gc_ref)?;
             header.kind().matches(VMGcKind::ArrayRef)
         });
         let arrayref = gc_ref.as_arrayref_unchecked();
-        Ok(arrayref.len(store))
+        arrayref.len(store)
     }
 
     /// Get the values of this array's elements.
@@ -647,7 +649,7 @@ impl ArrayRef {
         let store = AutoAssertNoGc::new(store);
 
         let gc_ref = self.inner.try_gc_ref(&store)?;
-        let header = store.require_gc_store()?.header(gc_ref);
+        let header = store.require_gc_store()?.header(gc_ref)?;
         debug_assert!(header.kind().matches(VMGcKind::ArrayRef));
 
         let len = self._len(&store)?;
@@ -677,7 +679,7 @@ impl ArrayRef {
                     return None;
                 }
                 self.index += 1;
-                Some(self.arrayref._get(&mut self.store, i).unwrap())
+                self.arrayref._get(&mut self.store, i).ok()
             }
 
             #[inline]
@@ -700,7 +702,7 @@ impl ArrayRef {
     fn header<'a>(&self, store: &'a AutoAssertNoGc<'_>) -> Result<&'a VMGcHeader> {
         assert!(self.comes_from_same_store(&store));
         let gc_ref = self.inner.try_gc_ref(store)?;
-        Ok(store.require_gc_store()?.header(gc_ref))
+        Ok(store.require_gc_store()?.header(gc_ref)?)
     }
 
     fn arrayref<'a>(&self, store: &'a AutoAssertNoGc<'_>) -> Result<&'a VMArrayRef> {
@@ -755,12 +757,12 @@ impl ArrayRef {
         let arrayref = self.arrayref(store)?.unchecked_copy();
         let field_ty = self.field_ty(store)?;
         let layout = self.layout(store)?;
-        let len = arrayref.len(store);
+        let len = arrayref.len(store)?;
         ensure!(
             index < len,
             "index out of bounds: the length is {len} but the index is {index}"
         );
-        Ok(arrayref.read_elem(store, &layout, field_ty.element_type(), index))
+        arrayref.read_elem(store, &layout, field_ty.element_type(), index)
     }
 
     /// Set this array's `index`th element.
@@ -811,7 +813,7 @@ impl ArrayRef {
         let layout = self.layout(&store)?;
         let arrayref = self.arrayref(&store)?.unchecked_copy();
 
-        let len = arrayref.len(&store);
+        let len = arrayref.len(&store)?;
         ensure!(
             index < len,
             "index out of bounds: the length is {len} but the index is {index}"
@@ -822,7 +824,7 @@ impl ArrayRef {
 
     pub(crate) fn type_index(&self, store: &StoreOpaque) -> Result<VMSharedTypeIndex> {
         let gc_ref = self.inner.try_gc_ref(store)?;
-        let header = store.require_gc_store()?.header(gc_ref);
+        let header = store.require_gc_store()?.header(gc_ref)?;
         debug_assert!(header.kind().matches(VMGcKind::ArrayRef));
         Ok(header.ty().expect("arrayrefs should have concrete types"))
     }

crates/wasmtime/src/runtime/gc/enabled/eqref.rs

@@ -2,7 +2,7 @@
 #![cfg(feature = "gc")]
 use crate::{
     AnyRef, ArrayRef, ArrayType, AsContext, AsContextMut, GcRefImpl, GcRootIndex, HeapType, I31,
-    OwnedRooted, RefType, Rooted, StructRef, StructType, ValRaw, ValType, WasmTy,
+    OwnedRooted, RefType, Rooted, StructRef, StructType, ValRaw, ValType, WasmTy, bail_bug,
     prelude::*,
     runtime::vm::VMGcRef,
     store::{AutoAssertNoGc, StoreOpaque},
@@ -168,6 +168,7 @@ impl EqRef {
                 || store
                     .unwrap_gc_store()
                     .header(&gc_ref)
+                    .unwrap()
                     .kind()
                     .matches(VMGcKind::EqRef)
         );
@@ -198,22 +199,26 @@ impl EqRef {
             return Ok(HeapType::I31);
         }
 
-        let header = store.require_gc_store()?.header(gc_ref);
+        let header = store.require_gc_store()?.header(gc_ref)?;
+        let ty = match header.ty() {
+            Some(ty) => ty,
+            None => bail_bug!("ty should be present"),
+        };
 
         if header.kind().matches(VMGcKind::StructRef) {
             return Ok(HeapType::ConcreteStruct(
-                StructType::from_shared_type_index(store.engine(), header.ty().unwrap()),
+                StructType::from_shared_type_index(store.engine(), ty),
             ));
         }
 
         if header.kind().matches(VMGcKind::ArrayRef) {
             return Ok(HeapType::ConcreteArray(ArrayType::from_shared_type_index(
                 store.engine(),
-                header.ty().unwrap(),
+                ty,
             )));
         }
 
-        unreachable!("no other kinds of `eqref`s")
+        bail_bug!("no other kinds of `eqref`s")
     }
 
     /// Does this `eqref` match the given type?
@@ -350,7 +355,7 @@ impl EqRef {
         Ok(!gc_ref.is_i31()
             && store
                 .require_gc_store()?
-                .kind(gc_ref)
+                .kind(gc_ref)?
                 .matches(VMGcKind::StructRef))
     }
 
@@ -418,7 +423,7 @@ impl EqRef {
         Ok(!gc_ref.is_i31()
             && store
                 .require_gc_store()?
-                .kind(gc_ref)
+                .kind(gc_ref)?
                 .matches(VMGcKind::ArrayRef))
     }